Page MenuHomeFreeBSD

Don't trim bsd_label
ClosedPublic

Authored by dougm on Aug 8 2019, 5:05 PM.

Details

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

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

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 ↗(On Diff #60601)

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 ↗(On Diff #60601)

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–2306 ↗(On Diff #60601)

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 ↗(On Diff #60607)

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 ↗(On Diff #60608)

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

94–99 ↗(On Diff #60608)

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 ↗(On Diff #60608)

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
259 ↗(On Diff #60787)

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
ota_j.email.ne.jp added inline comments.
head/sys/vm/swap_pager.c
2421

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.