On Fri, Aug 13, 2021 at 04:20:34PM +0530, Chandan Babu R wrote:
> On 11 Aug 2021 at 17:58, "Darrick J. Wong" <djwong@xxxxxxxxxx> wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > The fsmap implementation for realtime devices uses the gap between
> > info->next_daddr and a free rtextent reported by xfs_rtalloc_query_range
> > to feed userspace fsmap records with an "unknown" owner. We use this
> > trick to report to userspace when the last rtextent in the filesystem is
> > in use by synthesizing a null rmap record starting at the next block
> > after the query range.
> >
> > Unfortunately, there's a minor accounting bug in the way that we
> > construct the null rmap record. Originally, ahigh.ar_startext contains
> > the last rtextent for which the user wants records. It's entirely
> > possible that number is beyond the end of the rt volume, so the location
> > synthesized rmap record /must/ be constrained to the minimum of the high
> > key and the number of extents in the rt volume.
> >
>
> When the number of blocks on the realtime device is not an integral multiple
> of xfs_sb->sb_rextsize, ahigh.ar_startext can contain a value which is one
> more than the highest valid rtextent. Hence, without this patch, the last
> record reported to the userpace might contain an invalid upper bound. Assuming
> that my understanding is indeed correct,
Correct. Thanks for reviewing this!
--D
>
> Reviewed-by: Chandan Babu R <chandanrlinux@xxxxxxxxx>
>
> > Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_fsmap.c | 22 +++++++++++++++++-----
> > 1 file changed, 17 insertions(+), 5 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
> > index 7d0b09c1366e..a0e8ab58124b 100644
> > --- a/fs/xfs/xfs_fsmap.c
> > +++ b/fs/xfs/xfs_fsmap.c
> > @@ -523,27 +523,39 @@ xfs_getfsmap_rtdev_rtbitmap_query(
> > {
> > struct xfs_rtalloc_rec alow = { 0 };
> > struct xfs_rtalloc_rec ahigh = { 0 };
> > + struct xfs_mount *mp = tp->t_mountp;
> > int error;
> >
> > - xfs_ilock(tp->t_mountp->m_rbmip, XFS_ILOCK_SHARED);
> > + xfs_ilock(mp->m_rbmip, XFS_ILOCK_SHARED);
> >
> > + /*
> > + * Set up query parameters to return free extents covering the range we
> > + * want.
> > + */
> > alow.ar_startext = info->low.rm_startblock;
> > + do_div(alow.ar_startext, mp->m_sb.sb_rextsize);
> > +
> > ahigh.ar_startext = info->high.rm_startblock;
> > - do_div(alow.ar_startext, tp->t_mountp->m_sb.sb_rextsize);
> > - if (do_div(ahigh.ar_startext, tp->t_mountp->m_sb.sb_rextsize))
> > + if (do_div(ahigh.ar_startext, mp->m_sb.sb_rextsize))
> > ahigh.ar_startext++;
> > +
> > error = xfs_rtalloc_query_range(tp, &alow, &ahigh,
> > xfs_getfsmap_rtdev_rtbitmap_helper, info);
> > if (error)
> > goto err;
> >
> > - /* Report any gaps at the end of the rtbitmap */
> > + /*
> > + * Report any gaps at the end of the rtbitmap by simulating a null
> > + * rmap starting at the block after the end of the query range.
> > + */
> > info->last = true;
> > + ahigh.ar_startext = min(mp->m_sb.sb_rextents, ahigh.ar_startext);
> > +
> > error = xfs_getfsmap_rtdev_rtbitmap_helper(tp, &ahigh, info);
> > if (error)
> > goto err;
> > err:
> > - xfs_iunlock(tp->t_mountp->m_rbmip, XFS_ILOCK_SHARED);
> > + xfs_iunlock(mp->m_rbmip, XFS_ILOCK_SHARED);
> > return error;
> > }
> >
>
>
> --
> chandan
