Re: [PATCH 1/3] xfs: fix leak memory when xfs_attr_inactive fails

[Dave Chinner <david@fromorbit.com>]

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@fromorbit.com