Page MenuHomeFreeBSD

Modify rc.d/fsck to handle new status from fsck/fsck_ffs
ClosedPublic

Authored by dab on Mar 13 2018, 3:03 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sep 22 2024, 1:37 PM
Unknown Object (File)
Sep 19 2024, 8:41 PM
Unknown Object (File)
Sep 18 2024, 5:18 PM
Unknown Object (File)
Sep 18 2024, 1:21 AM
Unknown Object (File)
Sep 17 2024, 6:39 PM
Unknown Object (File)
Sep 8 2024, 10:51 PM
Unknown Object (File)
Sep 7 2024, 9:56 PM
Unknown Object (File)
Sep 7 2024, 12:04 PM

Details

Summary

r328013 introduced a new error code from fsck_ffs that indicates that it could not completely fix the file system; this happens when it prints the message PLEASE RERUN FSCK. However, this status can happen when fsck is run in "preen" mode and the rc.d/fsck script does not handle that error code. Modify rc.d/fsck so that if "fsck -p" ("preen") returns the new status code (16) it will run "fsck -y", as it currently does for a status code of 8 (the "standard error exit").

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Since fsck_y_enable is "NO" by default, this leaves us with fragile out-of-the-box behaviour. I would really like us to try harder to not leave a system stuck in single-user mode.

Ian reported in the thread that -R has no effect if -p is specified, but I'm not seeing how that's true (and that behaviour doesn't really make sense if it is true).

Rodney's claim that we wouldn't want to run fsck_ffs -p multiple times if requested also doesn't make sense to me: why would you be ok with running it once, unprompted, but not multiple times?

Ian reported in the thread that -R has no effect if -p is specified, but I'm not seeing how that's true (and that behaviour doesn't really make sense if it is true).

I just looked at the code again, and I think maybe I was confused between ERESTART vs ERERUN or something, because now it looks to me like the -R option does restart internally in preen mode. Sorry for the misinformation.

Given that, I think "fsck -p -T ffs:-R" should be the standard set of flags in defaults/rc.conf. If preen is safe to run once, it should be safe to run multiple times as needed.

Since fsck_y_enable is "NO" by default, this leaves us with fragile out-of-the-box behaviour. I would really like us to try harder to not leave a system stuck in single-user mode.

Ian reported in the thread that -R has no effect if -p is specified, but I'm not seeing how that's true (and that behaviour doesn't really make sense if it is true).

Rodney's claim that we wouldn't want to run fsck_ffs -p multiple times if requested also doesn't make sense to me: why would you be ok with running it once, unprompted, but not multiple times?

I think you misunderstood my assertions. My assertion was "I" do not want to run fsck multple times if fsck -p failed. If fsck-p failed "I" want to investigate. I do not want this ability out of my control.
The "I" above can easily be extended to all the clients I support. Your patch above is just fine with me, as "I" would never ever allow a fsck -y to run on a system that this could put data at risk, and
this patch leaves me that knob.

If fsck -p internally does a restart I do have some concerns, as this usually is indicative of a decaying situation. Also have any of you considered what is going on in the "failing drive" model,
continuing to hammer away on a drive with a minor issue can quickly change that to a major issue. IIRC fsck does not ever see or deal with hardware layer drive errors, it should, and it
should be an immediate bail out without options telling it otherwise.

This revision is now accepted and ready to land.Mar 13 2018, 5:21 PM

If fsck -p internally does a restart I do have some concerns, as this usually is indicative of a decaying situation.

I've hit this problem dozens of times in my setup, and the second invocation does nothing but mark the superblock clean. I don't agree with your claim.

Also have any of you considered what is going on in the "failing drive" model,
continuing to hammer away on a drive with a minor issue can quickly change that to a major issue. IIRC fsck does not ever see or deal with hardware layer drive errors, it should, and it
should be an immediate bail out without options telling it otherwise.

fsck won't restart if, e.g., a write() returns EIO. It exits with status 8 in that case - see rwerror() in the fsck_ffs source code.

In D14679#308387, @ian wrote:

Ian reported in the thread that -R has no effect if -p is specified, but I'm not seeing how that's true (and that behaviour doesn't really make sense if it is true).

I just looked at the code again, and I think maybe I was confused between ERESTART vs ERERUN or something, because now it looks to me like the -R option does restart internally in preen mode. Sorry for the misinformation.

Given that, I think "fsck -p -T ffs:-R" should be the standard set of flags in defaults/rc.conf. If preen is safe to run once, it should be safe to run multiple times as needed.

I am ok with fsck -p running a second pass IFF it was caused by changes made during the first pass that it needs to go back and clean up, NOT for any of the failures that normally lead to "Rerun fsck manually".
I am not sure I would support fsck -p doing more than 1 ERESTART, as this usually indicates a degrading situation and fsck is lost and not going to do the right thing in N passes.

If fsck -p internally does a restart I do have some concerns, as this usually is indicative of a decaying situation.

I've hit this problem dozens of times in my setup, and the second invocation does nothing but mark the superblock clean. I don't agree with your claim.

Humm, if alls it does in the second pass is mark the super block clean, why did it need to make the second pass? This sounds like a bug in the code.
I am fine with the idea of fsck -p running a single, but no more, restart as I stated elsewhere, this would take care of your issue.

Also have any of you considered what is going on in the "failing drive" model,
continuing to hammer away on a drive with a minor issue can quickly change that to a major issue. IIRC fsck does not ever see or deal with hardware layer drive errors, it should, and it
should be an immediate bail out without options telling it otherwise.

fsck won't restart if, e.g., a write() returns EIO. It exits with status 8 in that case - see rwerror() in the fsck_ffs source code.

I know that for some period between 5.4 and 11 due to the transistion to cam layer for ata that many errors never made it back to userland code. I have not retested for this failure, but it is one of the reasons I still do all of my disk repair work on 5.4p8 as when I tried to move forward the undelivered to userland errors drove me nuts. dd conv=noerror,sync just does not work in that situation as it never sees the error. Nor do most of my other tooling.

I do understand that fsck can bail out, I am uncertain that drive layer errors are making it to fsck in a timely, or if at all, manner.

Please try to understand that your trying to get your system up at almost any cost, and I am trying to protect user data at any cost, these have different needs.
I'll support any knobs you want to add to get to your goal, just please do not remove or change any knobs that prevent me from getting to my goal.

[...]

I'll support any knobs you want to add to get to your goal, just please do not remove or change any knobs that prevent me from getting to my goal.

It sounds like maybe what we need is to add support for an "fsck_p_flags" rcvar that gets used on fsck -p runs. We can put fsck_p_flags="-T ffs:-R" in /etc/defaults/rc.conf so that by default we fully preen a filesystem automatically, but users/admins can override it to turn off that default as needed.

Right now fsck_ffs is hardcoded internally to 10 retries, and I think that number was probably chosen arbitrarily when the feature was added, just to prevent looping forever if something unforseen happens. Maybe it could be enhanced so that -R takes an optional retry count. (IMO, 2 retries would make a better default.)

In D14679#308469, @ian wrote:

[...]

I'll support any knobs you want to add to get to your goal, just please do not remove or change any knobs that prevent me from getting to my goal.

It sounds like maybe what we need is to add support for an "fsck_p_flags" rcvar that gets used on fsck -p runs. We can put fsck_p_flags="-T ffs:-R" in /etc/defaults/rc.conf so that by default we fully preen a filesystem automatically, but users/admins can override it to turn off that default as needed.

Prefer to not change 25 year old behavior in this area, with softupdates, which is what users should be on by default, this is all pretty much a moot point anyway.
If your formatting your own systems and running without soft updates you *might* want fsck_p_flags with a ffs:-R in it.

I do support adding a fsck_p_flags though, so that if you want -T ffs:-R you can do that.

Right now fsck_ffs is hardcoded internally to 10 retries, and I think that number was probably chosen arbitrarily when the feature was added, just to prevent looping forever if something unforseen happens. Maybe it could be enhanced so that -R takes an optional retry count. (IMO, 2 retries would make a better default.)

Ouch, yes, in a "failing drive" situation this is just going to pound the heads into the ground, is there an abort on hardware I/O error? On -p I would say, and marks data seems to support, that 1 initial pass and 1 retry pass and it should be good, beyond that I do not see fsck -p fixing anything.

What about defaults of 1 retry for fsck -p, and 2 retries for other fsk, and adding your -R n option to allow either to be over ridden, including the deadly fsck -R 0 #go for ever or until clean.

A couple of points that I would like to add to this discussion:

  1. The rerunning of fsck can happen when certain cleanups are needed that may undo some earlier changes. For example when a disconnected directory is added to lost+found, previously calculated link counts for the directory that it used to be in may need to be adjusted. Rather than add a lot of code to fsck to deal with these rare cases, it is easier to just rerun it. Worst case scenario, it could happen once for each level of directory hierarchy in the filesystem. In practice I have only ever been able to make it loop twice with carefully constructed damage to the filesystem. There are many places where a change could cause the need for a rerun (and thus it is requested) even though it actually does not which is why the most common examples where it reruns, the result is simply to mark the filesystem clean.
  1. Running fsck -y is a last-ditch effort to recover a filesystem and IMO should never be done automatically, so not sure why that is even an option.
  1. Fsck bails out on any EIO or ENODEV errors. Warner Losh (imp@) at Netflix has been working to clean up the I/O subsystem so that these errors are consistently returned. So failing disks should be readily detected in the not too distant future.
  1. Fsck bails out on any EIO or ENODEV errors. Warner Losh (imp@) at Netflix has been working to clean up the I/O subsystem so that these errors are consistently returned. So failing disks should be readily detected in the not too distant future.

We've hired someone to do a lot of this work. I'm just overseeing until he gets sea legs... :) We've set the ambitious goal of being able to pull a drive any time and have the system not panic. Today, there's many lurking lifetime / dependency issues that make it hard to survive. Too many thins get destroyed a bit prematurely while BIOs are still in flight, and/or don't know how to flush dirty buffers that have no backing store any longer...

A couple of points that I would like to add to this discussion:

  1. The rerunning of fsck can happen when certain cleanups are needed that may undo some earlier changes. For example when a disconnected directory is added to lost+found, previously calculated link counts for the directory that it used to be in may need to be adjusted. Rather than add a lot of code to fsck to deal with these rare cases, it is easier to just rerun it. Worst case scenario, it could happen once for each level of directory hierarchy in the filesystem. In practice I have only ever been able to make it loop twice with carefully constructed damage to the filesystem. There are many places where a change could cause the need for a rerun (and thus it is requested) even though it actually does not which is why the most common examples where it reruns, the result is simply to mark the filesystem clean.

Am I reading this correctly in what I get is that 2 passes over the file system and we should be clean in most cases? That would agree with my experiences.
Also has the expansion of lost+found been fixed, iirc there use to be a problem here in that if you filled lost+found you more or less got stuck in a loop?

  1. Running fsck -y is a last-ditch effort to recover a filesystem and IMO should never be done automatically, so not sure why that is even an option.

I tried to discourage it when it went in, but was unsuccessful.

  1. Fsck bails out on any EIO or ENODEV errors. Warner Losh (imp@) at Netflix has been working to clean up the I/O subsystem so that these errors are consistently returned. So failing disks should be readily detected in the not too distant future.

Its this current situation that is troublesome for me. While imaging a drive last night in an attempt to move some of my tooling to -current I found that dd does not stop when the drive goes offline since it never gets the EOF that stops a dd conv=noerror. I am making changes so that dd is passed a total expected image size to correct for this.

FINALLY: I see no reason not to commit the change in this review, it simply adds the 16) error in the proper place, and has no other effects. This is needed to prevent the error from being ignored. If you agree could you check off on the review,
Thanks.

  1. Running fsck -y is a last-ditch effort to recover a filesystem and IMO should never be done automatically, so not sure why that is even an option.

I tried to discourage it when it went in, but was unsuccessful.

All the world is not a server containing critical data or a desktop containing your only copy of the family photo album. Sometimes the world is an embedded device contained within an even larger embedded system, all of it installed on some remote mountaintop. If fsck -y fails to recover such a system and damages the data even more, the result is exactly the same as if fsck -p exits with a code that leaves your data unmolested but dumps you into single-user mode: the system is bricked.

I concur that this change should be made.

FINALLY: I see no reason not to commit the change in this review, it simply adds the 16) error in the proper place, and has no other effects. This is needed to prevent the error from being ignored. If you agree could you check off on the review,
Thanks.

The change has no effect unless fsck_y_enable is set, in which case we perform a potentially destructive operation (fsck -y) in a situation where fsck -p probably suffices. I don't object to it, but don't see the value in it either.

FINALLY: I see no reason not to commit the change in this review, it simply adds the 16) error in the proper place, and has no other effects. This is needed to prevent the error from being ignored. If you agree could you check off on the review,
Thanks.

The change has no effect unless fsck_y_enable is set, in which case we perform a potentially destructive operation (fsck -y) in a situation where fsck -p probably suffices. I don't object to it, but don't see the value in it either.

The value is that without this, you don't get the fsck -y you requested by setting fsck_y_enable to true, you unconditionally drop to single-user mode if preen returns 16. This makes the concept of "try -y if -p fails" work again.

Retrying the presumably-safer preen operation before falling back to fsck -y might be preferable, and an fsck_p_flags option might be a good way to make that happen, but that's separate work that needn't hold up this change.

In D14679#308919, @ian wrote:

The value is that without this, you don't get the fsck -y you requested by setting fsck_y_enable to true, you unconditionally drop to single-user mode if preen returns 16. This makes the concept of "try -y if -p fails" work again.

Retrying the presumably-safer preen operation before falling back to fsck -y might be preferable, and an fsck_p_flags option might be a good way to make that happen, but that's separate work that needn't hold up this change.

I see, thanks.

This revision was automatically updated to reflect the committed changes.