Page MenuHomeFreeBSD
Differential D4449

Add option to disable inital userland sync to reboot(8)
ClosedPublic
Actions

Authored by smh on Dec 8 2015, 11:22 PM.
Tags
None
Referenced Files
F97957394: D4449.id10945.diff
Tue, Oct 1, 8:47 PM
Unknown Object (File)
Tue, Oct 1, 1:49 PM
Unknown Object (File)
Thu, Sep 26, 9:37 PM
Unknown Object (File)
Thu, Sep 26, 6:19 PM
Unknown Object (File)
Thu, Sep 26, 3:05 PM
Unknown Object (File)
Sun, Sep 22, 12:35 PM
Unknown Object (File)
Tue, Sep 17, 10:13 AM
Unknown Object (File)
Wed, Sep 11, 4:21 AM
View All Files
Subscribers
bjk
imp
uqs
wblock

Details

Reviewers
bjk
trasz
Group Reviewers
manpages
Commits
rS292947: Fix use of uninitialised Nflag
rS292266: Add flag to disable inital reboot(8) userland sync
Summary

It can be useful to bypass the userland sync(2) during reboot(8) while still allow the kernel sync to occur.

An example of this is when rebooting with disconnected iSCSI sessions which would otherwise cause the reboot to hang on BIOs that will never complete.

Add the -N option to reboot etc that allows the userland syncs to be skipped but still process the kernel sync during the reboot(2) syscall.

Test Plan

This depends on https://reviews.freebsd.org/D4429
reboot(8) a system with oustanding BIOs due to disconnected iSCSI session backing a mounted FS on a debug kernel.

  1. Without any options the system will hang due to oustanding BIOs
  2. With -n either a panic or hang will occur.
  3. With -N a clean reboot will occur.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1640
Build 1646: arc lint + arc unit

Event Timeline

smh updated this revision to Diff 10945.Dec 8 2015, 11:22 PM
smh retitled this revision from to Add option to disable inital userland sync to reboot(8).
smh updated this object.
smh edited the test plan for this revision. (Show Details)
Herald added a reviewer: manpages. · View Herald TranscriptDec 8 2015, 11:22 PM
Herald added a subscriber: imp. · View Herald Transcript
smh added a reviewer: trasz.Dec 8 2015, 11:23 PM
bjk added a subscriber: bjk.Dec 9 2015, 5:03 PM
bjk added inline comments.
sbin/reboot/reboot.8
102

I think I would hyphenate "best-effort" and possibly also put it within .Dq.

103

comma both before and after "e.g.".

smh updated this revision to Diff 10993.Dec 9 2015, 6:39 PM

Updated as per bjk's feedback.

Herald edited edge metadata. · View Herald TranscriptDec 9 2015, 6:39 PM
smh marked 2 inline comments as done.Dec 9 2015, 6:40 PM
wblock added a subscriber: wblock.Dec 10 2015, 3:22 AM
wblock added inline comments.
sbin/reboot/reboot.8
104

This sentence is long and has a couple of pauses. My person preference is to avoid Latin when possible. How about:

reboot when devices might be unavailable.
This can happen when devices have been disconnected, such as with
.Xr iscsi 4 .
smh updated this revision to Diff 11052.Dec 10 2015, 9:12 PM
smh edited edge metadata.

Use wblock's words.

Herald edited edge metadata. · View Herald TranscriptDec 10 2015, 9:12 PM
smh marked an inline comment as done.Dec 10 2015, 9:13 PM
smh added a comment.Dec 14 2015, 2:29 PM

Hi guys is everyone happy with this current version now?

bjk accepted this revision.Dec 14 2015, 7:45 PM
bjk added a reviewer: bjk.

Seems okay here.

Unless we want to add a note about -n and -N being mutually incompatible, but one would hope that the existing descriptions suffice to indicate that.

This revision is now accepted and ready to land.Dec 14 2015, 7:45 PM
smh updated this revision to Diff 11246.Dec 14 2015, 8:34 PM
smh edited edge metadata.

Added check for -N and -n to prevent confusion based on bjk's feedback.

This revision now requires review to proceed.Dec 14 2015, 8:34 PM
smh added a comment.Dec 14 2015, 10:14 PM

If you can confirm you're still ok with addition of the "-N cannot be used with -n" bjk, that would be appreciated

bjk accepted this revision.Dec 14 2015, 10:19 PM
bjk edited edge metadata.

That looks fine, thanks.

This revision is now accepted and ready to land.Dec 14 2015, 10:19 PM
Closed by commit rS292266: Add flag to disable inital reboot(8) userland sync (authored by smh). · Explain WhyDec 15 2015, 2:17 PM
This revision was automatically updated to reflect the committed changes.
uqs reopened this revision.Dec 29 2015, 12:24 PM
uqs added a subscriber: uqs.

Nflag is used un-initialized, please fix.

http://scan.spoerlein.net/scan-build/freebsd-head/WORLD/2015-12-27-amd64/report-09f0b7.html#EndPath

This revision is now accepted and ready to land.Dec 29 2015, 12:24 PM
uqs added inline comments.Dec 29 2015, 12:24 PM
head/sbin/reboot/reboot.c
79 ↗(On Diff #11308)

You need to also set Nflag = 0 here

Closed by commit rS292947: Fix use of uninitialised Nflag (authored by smh). · Explain WhyDec 30 2015, 2:58 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sbin/
reboot/
20 lines
10 lines

Diff 11246

View Options

sbin/reboot/reboot.8

Loading...
View Options

sbin/reboot/reboot.c

Loading...