Re: [PATCH] xfs: detect misaligned rtinherit directory extent size hints

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

On Thu, Jul 15, 2021 at 10:06:04AM +1000, Dave Chinner wrote:
> On Thu, Jul 15, 2021 at 09:50:49AM +1000, Dave Chinner wrote:
> > On Wed, Jul 14, 2021 at 02:35:42PM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > If we encounter a directory that has been configured to pass on an
> > > extent size hint to a new realtime file and the hint isn't an integer
> > > multiple of the rt extent size, we should flag the hint for
> > > administrative review because that is a misconfiguration (that other
> > > parts of the kernel will fix automatically).
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > >  fs/xfs/scrub/inode.c |   18 ++++++++++++++++--
> > >  1 file changed, 16 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/fs/xfs/scrub/inode.c b/fs/xfs/scrub/inode.c
> > > index 61f90b2c9430..76fbc7ca4cec 100644
> > > --- a/fs/xfs/scrub/inode.c
> > > +++ b/fs/xfs/scrub/inode.c
> > > @@ -73,11 +73,25 @@ xchk_inode_extsize(
> > >  	uint16_t		flags)
> > >  {
> > >  	xfs_failaddr_t		fa;
> > > +	uint32_t		value = be32_to_cpu(dip->di_extsize);
> > >  
> > > -	fa = xfs_inode_validate_extsize(sc->mp, be32_to_cpu(dip->di_extsize),
> > > -			mode, flags);
> > > +	fa = xfs_inode_validate_extsize(sc->mp, value, mode, flags);
> > >  	if (fa)
> > >  		xchk_ino_set_corrupt(sc, ino);
> > > +
> > > +	/*
> > > +	 * XFS allows a sysadmin to change the rt extent size when adding a rt
> > > +	 * section to a filesystem after formatting.  If there are any
> > > +	 * directories with extszinherit and rtinherit set, the hint could
> > > +	 * become misaligned with the new rextsize.  The verifier doesn't check
> > > +	 * this, because we allow rtinherit directories even without an rt
> > > +	 * device.  Flag this as an administrative warning since we will clean
> > > +	 * this up eventually.
> > > +	 */
> > > +	if ((flags & XFS_DIFLAG_RTINHERIT) &&
> > > +	    (flags & XFS_DIFLAG_EXTSZINHERIT) &&
> > > +	    value % sc->mp->m_sb.sb_rextsize > 0)
> > > +		xchk_ino_set_warning(sc, ino);
> > >  }
> > >  
> > >  /*
> > 
> > Looks good. A warning seems appropriate for scrub in this corner
> > case...
> 
> so...
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Cool, thanks for the review!

--D

> 
> -- 
> Dave Chinner
> david@fromorbit.com