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
Unknown Object (File)
Fri, Jan 17, 4:36 PM
Unknown Object (File)
Mon, Jan 13, 7:24 PM
Unknown Object (File)
Sun, Jan 12, 7:12 AM
Unknown Object (File)
Fri, Jan 3, 6:06 PM
Unknown Object (File)
Thu, Dec 26, 4:21 PM
Unknown Object (File)
Dec 20 2024, 12:40 AM
Unknown Object (File)
Dec 1 2024, 3:49 PM
Unknown Object (File)
Nov 30 2024, 12:41 AM
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

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
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
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

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
head/
sbin/
swapon/
12 lines
5 lines
share/
man/
man5/
13 lines
sys/
vm/
8 lines

Diff 60813

View Options

head/sbin/swapon/swapon.8

Loading...
View Options

head/sbin/swapon/swapon.c

Loading...
View Options

head/share/man/man5/fstab.5

Loading...
View Options

head/sys/vm/swap_pager.c

Loading...