Re: [PATCH 6/9] xfs: set inode sick state flags when we zap either ondisk fork

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Dec 06, 2023 at 09:58:58PM -0800, Christoph Hellwig wrote:
> On Wed, Dec 06, 2023 at 06:43:16PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Christoph asked for stronger protections against online repair zapping a
> > fork to get the inode to load vs. other threads trying to access the
> > partially repaired file.  Do this by adding a special "[DA]FORK_ZAPPED"
> > inode health flag whenever repair zaps a fork, and sprinkling checks for
> > that flag into the various file operations for things that don't like
> > handling an unexpected zero-extents fork.
> > 
> > In practice xfs_scrub will scrub and fix the forks almost immediately
> > after zapping them, so the window is very small.
> 
> This probably should be before the previous two patches, and the
> reordering seems easy enough.

Done.

> We should also have a blurb in the commit log and code that this flag
> right now is in-memory only and thus the zapped forks can leak through
> an unmount or crash.

Fixed.  The last paragraph now reads:

"In practice xfs_scrub will scrub and fix the forks almost immediately
after zapping them, so the window is very small.  However, if a crash or
unmount should occur, we can still detect these zapped inode forks by
looking for a zero-extents fork when data was expected."

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Actually, I found a couple more bugs -- the checks for
XFS_SICK_INO_*FORK_ZAPPED in bmap.c that control setting the CORRUPT
flag should not do that if we're revalidating after a repair.  I decided
they should also be hoisted up to the xchk_bmap caller to avoid
splitting the logic:

/* Scrub an inode's data fork. */
int
xchk_bmap_data(
	struct xfs_scrub	*sc)
{
	int			error;

	/* Ignore old state if we're revalidating after a repair. */
	if (!(sc->flags & XREP_ALREADY_FIXED) &&
	    xfs_inode_has_sickness(sc->ip, XFS_SICK_INO_DFORK_ZAPPED)) {
		xchk_ino_set_corrupt(sc, sc->ip->i_ino);
		return 0;
	}

	error = xchk_bmap(sc, XFS_DATA_FORK);
	if (error)
		return error;

	/* If the data fork is clean, it is clearly not zapped. */
	xchk_mark_healthy_if_clean(sc, XFS_SICK_INO_DFORK_ZAPPED);
	return 0;
}

--D