Page MenuHomeFreeBSD
Differential D21191

Don't trim bsd_label
ClosedPublic
Actions

Authored by dougm on Aug 8 2019, 5:05 PM.
Tags
None
Referenced Files
F133419819: D21191.id60591.diff
Sat, Oct 25, 4:21 PM
F133320399: D21191.id.diff
Fri, Oct 24, 10:08 PM
Unknown Object (File)
Fri, Oct 24, 3:15 PM
Unknown Object (File)
Thu, Oct 23, 8:48 PM
Unknown Object (File)
Mon, Oct 20, 11:16 AM
Unknown Object (File)
Sun, Oct 19, 2:06 AM
Unknown Object (File)
Sun, Oct 19, 2:06 AM
Unknown Object (File)
Sat, Oct 18, 8:24 PM
View All Files
Subscribers
imp
ota_j.email.ne.jp

Details

Reviewers
alc
markj
Group Reviewers
manpages
Commits
rS351064: swap_pager.c reserves 2 blocks for a bsd label. Change that 2 to the
Summary

Use BBSIZE / PAGE_SIZE, instead of 2, in swap_pager.c. Don't trim the first BBSIZE bytes, swapon_trim, in swapon.c.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

dougm created this revision.Aug 8 2019, 5:05 PM
markj added inline comments.Aug 8 2019, 7:37 PM
sys/vm/vm_param.h
127 ↗(On Diff #60591)

I believe you can use BBSIZE from sys/disklabel.h instead.

dougm updated this revision to Diff 60598.Aug 8 2019, 8:41 PM
dougm retitled this revision from name bsd_label_size, and don't trim bsd_label to Don't trim bsd_label.
dougm edited the summary of this revision. (Show Details)
markj accepted this revision.Aug 8 2019, 8:59 PM
This revision is now accepted and ready to land.Aug 8 2019, 8:59 PM
dougm updated this revision to Diff 60601.Aug 8 2019, 9:37 PM

Make associated manpage fixes, including a clumsy attempt to address a savecore issue.

This revision now requires review to proceed.Aug 8 2019, 9:37 PM
alc added inline comments.Aug 8 2019, 10:50 PM
sbin/swapon/swapon.8
94–97

The crash dump will still be deleted by the -E option. The late option will simply delay the execution of swapon(8) until after savecore(8) has copied the crash dump to another location.

alc added inline comments.Aug 8 2019, 10:55 PM
share/man/man5/fstab.5
250

I would revert back to using the word "blocks" here.

alc added inline comments.Aug 8 2019, 11:11 PM
sys/vm/swap_pager.c
2305–2307

Consider the possibility that PAGE_SIZE is greater than BBSIZE. (We still barely support one architecture, sparc64, where they are equal.) I think that howmany(BBSIZE, PAGE_SIZE) will "do the right thing."

dougm updated this revision to Diff 60607.Aug 9 2019, 3:00 AM

Apply reviewer suggestions.

alc added inline comments.Aug 9 2019, 4:28 AM
sys/vm/swap_pager.c
2324

This should also use howmany(BBSIZE, PAGE_SIZE).

dougm updated this revision to Diff 60608.Aug 9 2019, 4:32 AM

Use howmany in one more place.

markj accepted this revision.Aug 14 2019, 4:45 AM
This revision is now accepted and ready to land.Aug 14 2019, 4:45 AM
alc added inline comments.Aug 14 2019, 3:51 PM
sbin/swapon/swapon.8
93

This sentence has become long enough that I would recommend splitting it into two: "... command. This command marks all blocks ...

94–99

My recollection is that the style guidelines for man pages say that new sentences should start on their own line in the first column.
I would reword this as follows:

"This marking can erase a crash dump.
To delay
.Nm swapon
for a device until after
.Nm savecore
has copied the crash dump to another location, use the
.Dq
late
option.

share/man/man5/fstab.5
249

Ditto: "... command to the device. This command marks ...

dougm updated this revision to Diff 60782.Aug 14 2019, 4:07 PM

Apply alc comments.

This revision now requires review to proceed.Aug 14 2019, 4:07 PM
dougm updated this revision to Diff 60786.Aug 14 2019, 4:17 PM
dougm updated this revision to Diff 60787.Aug 14 2019, 4:31 PM

line wrap.

alc accepted this revision.Aug 14 2019, 4:36 PM
This revision is now accepted and ready to land.Aug 14 2019, 4:36 PM
alc added a comment.Aug 14 2019, 4:37 PM

I would recommend "MFC after: 3 days".

markj added inline comments.Aug 14 2019, 4:57 PM
share/man/man5/fstab.5
260

Shouldn't ".Dq late" be one line?

dougm updated this revision to Diff 60791.Aug 14 2019, 5:03 PM

.Dq late

This revision now requires review to proceed.Aug 14 2019, 5:03 PM
markj accepted this revision.Aug 14 2019, 5:07 PM
This revision is now accepted and ready to land.Aug 14 2019, 5:07 PM
alc accepted this revision.Aug 14 2019, 7:25 PM
Closed by commit rS351064: swap_pager.c reserves 2 blocks for a bsd label. Change that 2 to the (authored by dougm). · Explain WhyAug 15 2019, 2:31 AM
This revision was automatically updated to reflect the committed changes.
dougm added a commit: rS351064: swap_pager.c reserves 2 blocks for a bsd label. Change that 2 to the.
Herald added a reviewer: manpages. · View Herald TranscriptAug 15 2019, 2:31 AM
Herald added a subscriber: imp. · View Herald Transcript
ota_j.email.ne.jp added a subscriber: ota_j.email.ne.jp.Aug 15 2019, 5:34 AM
ota_j.email.ne.jp added inline comments.
head/sys/vm/swap_pager.c
2421 ↗(On Diff #60813)

Which one is a better value to pass here, 0 or howmany(BBSIZE, PAGE_SIZE) ?

I think behavior is the same regardless of the value given blist_free(sp->sw_blist, howmany(BBSIZE, PAGE_SIZE), nblks - howmany(BBSIZE, PAGE_SIZE)) was done.

Revision Contents
Changeset List

PathSize
sbin/
swapon/
8 lines
5 lines
share/
man/
man5/
2 lines
sys/
vm/
8 lines

Diff 60608

View Options

sbin/swapon/swapon.8

Loading...
View Options

sbin/swapon/swapon.c

Loading...
View Options

share/man/man5/fstab.5

Loading...
View Options

sys/vm/swap_pager.c

Loading...