On 2/9/21 1:43 PM, Darrick J. Wong wrote: > On Tue, Feb 09, 2021 at 02:14:22PM -0500, Brian Foster wrote: >> On Tue, Feb 09, 2021 at 10:35:42AM -0800, Darrick J. Wong wrote: >>> On Tue, Feb 09, 2021 at 12:20:59PM -0500, Brian Foster wrote: >>>> On Mon, Feb 08, 2021 at 08:10:44PM -0800, Darrick J. Wong wrote: >>>>> From: Darrick J. Wong <djwong@xxxxxxxxxx> >>>>> >>>>> There are a few places in xfs_repair's directory checking code where we >>>>> deliberately corrupt a directory entry as a sentinel to trigger a >>>>> correction in later repair phase. In the mean time, the filesystem is >>>>> inconsistent, so set the needsrepair flag to force a re-run of repair if >>>>> the system goes down. >>>>> >>>>> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx> >>>>> --- >>>> >>>> Hmm.. this seems orthogonal to the rest of the series. I'm sure we can >>>> come up with various additional uses for the bit, but it seems a little >>>> odd to me that repair might set it in some cases after a crash but not >>>> others (if the filesystem happens to already be corrupt, for example). >>> >>> <nod> Another option I thought of is to add a hook to the buffer cache >>> so that the first time anyone tries to bwrite a buffer (either directly >>> or via a delwri list or normal buffer cache writeback) we'll also set >>> needsrepair on the ondisk primary super. That would protect us against >>> other scenarios like crashing after writing a new AGF but before writing >>> the new AGI, where the fs is left in an indeterminate state. >>> >> >> Yeah, that _seems_ more appropriate to me. It's not immediately clear to >> me what the implementation should look like, but in general behavior >> that sets needsrepair on first modification and clears it as a final >> step sounds more practical. Then the behavior can be easily explained as >> "once repair starts on an fs, it must be completed before it is allowed >> to mount." I do think this should be lifted off for a followon series so >> we can make progress on the feature upgrade bits without growing more >> requirements and complexity.. > > Oh, definitely. I'll withdraw this patch for now in the interests of > getting everything else going for Eric. :) Noted, I'll drop this one for now, thanks. -Eric
