From: Brian Foster <bfoster@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 3/4] repair: use fs root ino for dummy parent value instead of zero
Date: Fri, 17 Jul 2020 07:57:39 -0400 [thread overview]
Message-ID: <20200717115739.GA58041@bfoster> (raw)
In-Reply-To: <20200716220605.GM2005@dread.disaster.area>
On Fri, Jul 17, 2020 at 08:06:05AM +1000, Dave Chinner wrote:
> On Thu, Jul 16, 2020 at 06:41:03AM -0400, Brian Foster wrote:
> > On Thu, Jul 16, 2020 at 08:22:16AM +1000, Dave Chinner wrote:
> > > On Wed, Jul 15, 2020 at 10:08:35AM -0400, Brian Foster wrote:
> > > > If a directory inode has an invalid parent ino on disk, repair
> > > > replaces the invalid value with a dummy value of zero in the buffer
> > > > and NULLFSINO in the in-core parent tracking. The zero value serves
> > > > no functional purpose as it is still an invalid value and the parent
> > > > must be repaired by phase 6 based on the in-core state before the
> > > > buffer can be written out. Instead, use the root fs inode number as
> > > > a catch all for invalid parent values so phase 6 doesn't have to
> > > > create custom verifier infrastructure just to work around this
> > > > behavior.
> > > >
> > > > Signed-off-by: Brian Foster <bfoster@redhat.com>
> > >
> > > Reasonale, but wouldn't it be better to use lost+found as the dummy
> > > parent inode (i.e. the orphanage inode)? Because if the parent can't
> > > be found and the inode reconnected correctly, we're going to put it
> > > in lost+found, anyway?
> > >
> >
> > That was my first thought when I originally wrote this, but there's
> > several reasons I didn't end up doing that. The orphanage isn't created
> > until much later in repair and only if we end up with orphaned inodes.
> > We'd have to change that in order to use a dummy parent inode number
> > that corresponds to a valid orphanage, and TBH I'm not even sure if it's
> > always going to be safe to expect an inode allocation to work at this
> > point in repair.
> >
> > Further, it's still too early to tell whether these directories are
> > orphaned because the directory scan in phase 6 can easily repair
> > missing/broken parent information. The scenarios I used to test this
> > functionality didn't involve the orphanage at all, so now we not only
> > need to change when/how the orphanage is created, but need to free it if
> > it ends up unused before we exit (which could be via any number of
> > do_error() calls before we ever get close to phase 6).
>
> Fair enough - can you please capture all this in the commit message
> to preserve the explanation of why the root inode was chosen and
> not lost+found?
>
Sure, v2 incoming...
Brian
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
>
next prev parent reply other threads:[~2020-07-17 11:57 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-15 14:08 [PATCH 0/4] xfsprogs: remove custom dir2 sf fork verifier from repair Brian Foster
2020-07-15 14:08 ` [PATCH 1/4] repair: set the in-core inode parent in phase 3 Brian Foster
2020-07-15 18:42 ` Christoph Hellwig
2020-07-21 0:57 ` Darrick J. Wong
2020-07-15 14:08 ` [PATCH 2/4] repair: don't double check dir2 sf parent in phase 4 Brian Foster
2020-07-15 18:43 ` Christoph Hellwig
2020-07-15 23:54 ` Darrick J. Wong
2020-07-16 10:39 ` Brian Foster
2020-07-21 0:55 ` Darrick J. Wong
2020-07-15 14:08 ` [PATCH 3/4] repair: use fs root ino for dummy parent value instead of zero Brian Foster
2020-07-15 18:44 ` Christoph Hellwig
2020-07-15 22:22 ` Dave Chinner
2020-07-16 10:41 ` Brian Foster
2020-07-16 22:06 ` Dave Chinner
2020-07-17 11:57 ` Brian Foster [this message]
2020-07-17 11:59 ` [PATCH v2] repair: use fs rootino " Brian Foster
2020-07-20 3:21 ` Dave Chinner
2020-07-21 0:47 ` Darrick J. Wong
2020-07-15 14:08 ` [PATCH 4/4] repair: remove custom dir2 sf fork verifier from phase6 Brian Foster
2020-07-15 18:44 ` Christoph Hellwig
2020-07-21 0:47 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200717115739.GA58041@bfoster \
--to=bfoster@redhat.com \
--cc=david@fromorbit.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).