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@xxxxxxxxxx> > > > > > > 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@xxxxxxxxxxxxx >
