Page MenuHomeFreeBSD

Don't trim bsd_label
ClosedPublic

Authored by dougm on Aug 8 2019, 5:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Mar 22, 6:24 PM
Unknown Object (File)
Fri, Mar 22, 6:24 PM
Unknown Object (File)
Fri, Mar 22, 6:24 PM
Unknown Object (File)
Fri, Mar 22, 6:24 PM
Unknown Object (File)
Fri, Mar 22, 6:24 PM
Unknown Object (File)
Fri, Mar 22, 6:23 PM
Unknown Object (File)
Fri, Mar 22, 6:23 PM
Unknown Object (File)
Fri, Mar 22, 6:23 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

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sys/vm/vm_param.h
127 ↗(On Diff #60591)

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

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)
This revision is now accepted and ready to land.Aug 8 2019, 8:59 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
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.

share/man/man5/fstab.5
250 ↗(On Diff #60601)

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

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."

Apply reviewer suggestions.

sys/vm/swap_pager.c
2324

This should also use howmany(BBSIZE, PAGE_SIZE).

Use howmany in one more place.

This revision is now accepted and ready to land.Aug 14 2019, 4:45 AM
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 ...

This revision now requires review to proceed.Aug 14 2019, 4:07 PM
This revision is now accepted and ready to land.Aug 14 2019, 4:36 PM

I would recommend "MFC after: 3 days".

share/man/man5/fstab.5
259 ↗(On Diff #60787)

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

This revision now requires review to proceed.Aug 14 2019, 5:03 PM
This revision is now accepted and ready to land.Aug 14 2019, 5:07 PM
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.