Re: [PATCH] xfs: bump INUMBERS cursor correctly in xfs_inumbers_walk

Brian Foster
2019-07-09T14:25:41Z

On Tue, Jul 09, 2019 at 07:22:26AM -0700, Darrick J. Wong wrote:
> On Tue, Jul 09, 2019 at 06:59:43AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> >
> > There's a subtle unit conversion error when we increment the INUMBERS
> > cursor at the end of xfs_inumbers_walk. If there's an inode chunk at
> > the very end of the AG /and/ the AG size is a perfect power of two, that
> > means we can have inodes, that means that the startino of that last
>
> "...is a perfect power of two, the startino of that last chunk..."
>

Yeah, was going to point out this looked like some stale/spurious
text... :P

> --D
>
> > chunk (which is in units of AG inodes) will be 63 less than (1 <<
> > agino_log). If we add XFS_INODES_PER_CHUNK to the startino, we end up
> > with a startino that's larger than (1 << agino_log) and when we convert
> > that back to fs inode units we'll rip off that upper bit and wind up
> > back at the start of the AG.
> >
> > Fix this by converting to units of fs inodes before adding
> > XFS_INODES_PER_CHUNK so that we'll harmlessly end up pointing to the
> > next AG.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---

Otherwise looks good:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> > fs/xfs/xfs_itable.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_itable.c b/fs/xfs/xfs_itable.c
> > index cda8ae94480c..a8a06bb78ea8 100644
> > --- a/fs/xfs/xfs_itable.c
> > +++ b/fs/xfs/xfs_itable.c
> > @@ -338,15 +338,14 @@ xfs_inumbers_walk(
> > .xi_version = XFS_INUMBERS_VERSION_V5,
> > };
> > struct xfs_inumbers_chunk *ic = data;
> > - xfs_agino_t agino;
> > int error;
> >
> > error = ic->formatter(ic->breq, &inogrp);
> > if (error && error != XFS_IBULK_ABORT)
> > return error;
> >
> > - agino = irec->ir_startino + XFS_INODES_PER_CHUNK;
> > - ic->breq->startino = XFS_AGINO_TO_INO(mp, agno, agino);
> > + ic->breq->startino = XFS_AGINO_TO_INO(mp, agno, irec->ir_startino) +
> > + XFS_INODES_PER_CHUNK;
> > return error;
> > }
> >