Bug 256746

Summary:	fsck_ffs completely broken for background fsck
Product:	Base System	Reporter:	Andrew "RhodiumToad" Gierth <andrew>
Component:	bin	Assignee:	Robert Wing <rew>
Status:	Closed FIXED
Severity:	Affects Many People	CC:	chris, fs, grahamperrin, mckusick, rew
Priority:	---	Flags:	koobs: mfc-stable13+
Version:	13.0-RELEASE
Hardware:	Any
OS:	Any
URL:	https://www.freebsd.org/releases/13.1R/relnotes/#storage-ufs
See Also:	https://reviews.freebsd.org/D35212

Description Andrew "RhodiumToad" Gierth 2021-06-21 12:07:54 UTC

Commit 5cc52631b3b88 completely breaks background fsck by trying to open the device for writing (which is of course disallowed because it is mounted) too early in the process. This affects 13.0 release, 13-stable and current.

To reproduce: have any filesystem other than / with softupdates enabled but not journalling; force a reboot with for example reboot -qn when the filesystem is mounted and dirty; observe that in /var/log/messages 60 seconds after booting a message like this shows up:

Jun 21 11:40:48 hel fsck[784]: /dev/vtbd1: NO WRITE ACCESS
Jun 21 11:40:48 hel fsck[784]: /dev/vtbd1: UNEXPECTED INCONSISTENCY; RUN fsck MANUALLY.
Jun 21 11:40:48 hel fsck[784]: /dev/vtbd1: CANNOT SET FS_NEEDSFSCK FLAG

The code at fsck_ffs/main.c:298 should probably be inside the FS_GJOURNAL condition, or otherwise refactored; it's obviously completely wrong as it stands. (Note that -p sets ckclean, and skipclean is set by default).

Comment 1 Kirk McKusick freebsd_committer

2021-06-21 12:38:09 UTC

Thanks for breaking out this bug separately. I will investigate it and report back.

Comment 2 Graham Perrin freebsd_committer

2021-06-22 10:12:22 UTC

Thank you, both. 

(In reply to andrew from comment #0)

<https://cgit.freebsd.org/src/commit/?id=5cc52631b3b88dfc36d8049dc8bece8573c5f9af>

Re: the most recent commit, line 298 is at
<https://cgit.freebsd.org/src/tree/sbin/fsck_ffs/main.c?id=441e69e419effac0225a45f4cdb948280b8ce5ab#n298>

Comment 3 Robert Wing freebsd_committer

2021-06-23 22:30:58 UTC

proposed fix: https://reviews.freebsd.org/D30880

Comment 4 Kirk McKusick freebsd_committer

2021-06-30 00:31:15 UTC

(In reply to Robert Wing from comment #3)
Your proposed fix in https://reviews.freebsd.org/D30880 looks good.

Comment 5 commit-hook freebsd_committer

2021-07-11 20:59:16 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=0c5a59252c8e7b80b98521ebc23a415a05ff9594

commit 0c5a59252c8e7b80b98521ebc23a415a05ff9594
Author:     Robert Wing <rew@FreeBSD.org>
AuthorDate: 2021-07-11 20:47:27 +0000
Commit:     Robert Wing <rew@FreeBSD.org>
CommitDate: 2021-07-11 20:47:27 +0000

    fsck_ffs: fix background fsck in preen mode

    Background checks are only allowed for mounted filesystems - don't try
    to open the device for writing when performing a background check.

    While here, remove a debugging printf that's commented out.

    PR:             256746
    Fixes:          5cc52631b3b88dfc36d8049dc8bece8573c5f9af
    Reviewed by:    mckusick
    MFC After:      1 week
    Differential Revision:  https://reviews.freebsd.org/D30880

 sbin/fsck_ffs/main.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comment 6 commit-hook freebsd_committer

2021-07-19 18:18:02 UTC

A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=fb2feceac34cc9c3fb47ba4a7b0ca31637f8fdf0

commit fb2feceac34cc9c3fb47ba4a7b0ca31637f8fdf0
Author:     Robert Wing <rew@FreeBSD.org>
AuthorDate: 2021-07-11 20:47:27 +0000
Commit:     Robert Wing <rew@FreeBSD.org>
CommitDate: 2021-07-19 18:06:53 +0000

    fsck_ffs: fix background fsck in preen mode

    Background checks are only allowed for mounted filesystems - don't try
    to open the device for writing when performing a background check.

    While here, remove a debugging printf that's commented out.

    PR:             256746
    Fixes:          5cc52631b3b88dfc36d8049dc8bece8573c5f9af
    Reviewed by:    mckusick
    Differential Revision:  https://reviews.freebsd.org/D30880

    (cherry picked from commit 0c5a59252c8e7b80b98521ebc23a415a05ff9594)

 sbin/fsck_ffs/main.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Comment 7 Graham Perrin freebsd_committer

2021-09-12 08:32:21 UTC

Please, when might this be engineered for release?

<https://cgit.freebsd.org/src/log/sbin/fsck_ffs/main.c?h=releng/13.0> the most recent commit was in January. 

Thanks

Comment 8 Kirk McKusick freebsd_committer

2021-09-15 04:41:22 UTC

(In reply to Graham Perrin from comment #7)
The bug has been MFC'ed (merged) into 13-stable which is available now.
The next engineered release will be 13.1 which will contain this change.

Comment 9 Kubilay Kocak freebsd_committer

2022-02-15 22:41:36 UTC

^Triage: Assign to committer that resolved and track merge(s)

Comment 10 Graham Perrin freebsd_committer

2022-05-15 14:23:51 UTC

⚙ D35212 13.1-RELEASE note: fsck_ffs: background fsck 
<https://reviews.freebsd.org/D35212>

Comment 11 Kirk McKusick freebsd_committer

2022-05-15 16:32:03 UTC

(In reply to Graham Perrin from comment #10)
I approved your suggested addition to the 31.1 release notes.