On Sun, Oct 02, 2022 at 11:20:29AM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > xchk_get_inode is not quite the right function to be calling from the > inode scrubber setup function. The common get_inode function either > gets an inode and installs it in the scrub context, or it returns an > error code explaining what happened. This is acceptable for most file > scrubbers because it is not in their scope to fix corruptions in the > inode core and fork areas that cause iget to fail. > > Dealing with these problems is within the scope of the inode scrubber, > however. If iget fails with EFSCORRUPTED, we need to xchk_inode to flag > that as corruption. Since we can't get our hands on an incore inode, we > need to hold the AGI to prevent inode allocation activity so that > nothing changes in the inode metadata. > > Looking ahead to the inode core repair patches, we will also need to > hold the AGI buffer into xrep_inode so that we can make modifications to > the xfs_dinode structure without any other thread swooping in to > allocate or free the inode. > > Adapt the xchk_get_inode into xchk_setup_inode since this is a one-off > use case where the error codes we check for are a little different, and > the return state is much different from the common function. The code look fine, but... ... doesn't this mean that xchk_setup_inode() and xchk_get_inode() now are almost identical apart from the xchk_prepare_iscrub() bits? This kinda looks like a lot of duplicated but subtly different code - does xchk_get_inode() still need all that complexity if we are now doing it in xchk_setup_inode()? If it does, why does xchk_setup_inode() need to duplicate the code? Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx
