Report CG checksum mismatches.
ClosedPublic
Actions

Authored by imp on Jan 12 2018, 3:42 PM.

Details

Reviewers

mckusick
emaste
kib
scottl

Commits

rS327970: Report CG checksum mismatches. These errors are non-fatal. The

Summary

Previously, we'd silently fix CG mismatches. However, we see checksum
mismatch errors at runtime. In investigating them for our use case, we
couldn't find any checksum errors at rest (eg on reboot). Enable that
code for everybody to see if it's just us, or others have the same
issue.

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

imp created this revision.Jan 12 2018, 3:42 PM

Harbormaster completed remote builds in B14332: Diff 37868.Jan 12 2018, 3:42 PM

imp added reviewers: mckusick, emaste, kib, scottl.Jan 12 2018, 3:43 PM

See also PR 225110

imp added inline comments.Jan 12 2018, 3:47 PM

sbin/fsck_ffs/pass5.c
188 ↗	(On Diff #37868)	not sure if this should be fatal, or just a warning, please advise. pfatal isn't always fatal, so I'm not 100% clear on when it should be used.

FWIW I have disabled SUJ by default in the installer, in rS327890, until the underlying issue with SUJ + CG checksums is addressed.

Using pfatal means that fsck -p (preen mode) will fail causing a reboot to drop into single user mode with a request that you run fsck manually. If you use pwarn, then a message will be generated but the system will continue to multi-user. I believe that pwarn is more appropriate in this case since you are not in danger of corrupting your filesystem when the checksum is wrong.

Given the issues that have been raised by adding the checkhash, I am thinking that it may be better not to enable it by default. If it is (for now) only enabled optionally, then folks that want to test it can do so and the rest of the users do not have to deal with the testing shake-out. I want to add other checkhashes for things like superblocks, inodes and possibly indirect blocks. Once these are working as a whole, then we can expand them to the larger population. In particular, I want to add the ability to detect when a filesystem is run on an earlier kernel so that we will expect the checkhashes to be wrong. I have just one bit to work with here, so I want to defer adding this until I can use it to cover all the checkhashes. But I think that this ability to detect running on earlier kernels is important to have for the wider population of users.

swills added a subscriber: swills.Jan 14 2018, 1:40 AM

This revision was not accepted when it landed; it landed in state Needs Review.Jan 14 2018, 4:55 PM

Closed by commit rS327970: Report CG checksum mismatches. These errors are non-fatal. The (authored by imp). · Explain Why

This revision was automatically updated to reflect the committed changes.