On Fri, Apr 21, 2023 at 08:32:16AM -0700, Darrick J. Wong wrote: > On Fri, Apr 21, 2023 at 05:49:32PM +1000, Dave Chinner wrote: > > Yes, I agree the way xfs_inactive() and inodegc handles errors and > > cleanup needs improvement, but we've known this for a while now. But > > this doesn't change the fact that we currently need to be able to > > leak resources we can't access so we can continue to operate. It's > > fine for ASSERTs to fire on debug kernels in these situations - as > > developers we need to understand when these situations occur - but > > that doesn't mean the behaviour they are warning about needs to be > > fixed. It's just telling us that we are leaking stuff, it just > > doesn't know why. > > ...and should probably be logging the fact that the bad inode was > dropped on the floor and the sysadmin should go run a fsck tool of some > kind to fix the problems. Runtime corruption detection at the point of error injection already warns via the XFS_IS_CORRUPT() macro: if (XFS_IS_CORRUPT(mp, !xfs_ifork_has_extents(ifp)) || XFS_TEST_ERROR(false, mp, XFS_ERRTAG_BMAPIFORMAT)) return -EFSCORRUPTED; > > We have be waiting on having fine grained health infomration for > > inodes to be able to handle situations like this more gracefully. > > That code is being merged in 6.4, and it means that we know the > > It is? > > I didn't send you a pull request for > https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux.git/log/?h=indirect-health-reporting > > for 6.4. At some point I want to talk to you about the rest of online > fsck, but I'm taking a breather for the last week or two until LSFMM. Oops, I'm getting ahead of myself, aren't I? I was looking at a local review branch that has everything merged in, not the for-next branch I thought I was looking at... -Dave. -- Dave Chinner david@xxxxxxxxxxxxx
