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? Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
