Re: [PATCH] xfs: correct calculation for blockcount

["Darrick J. Wong" <djwong@xxxxxxxxxx>]

On Fri, Sep 08, 2023 at 06:18:52PM +0800, Shiyang Ruan wrote:
> Ping~
> 
> 在 2023/8/28 15:24, Shiyang Ruan 写道:
> > The blockcount, which means length, should be "end + 1 - start".  So,
> > add the missing "+1" here.
> > 
> > Fixes: 5cf32f63b0f4 ("xfs: fix the calculation for "end" and "length"")
> > Signed-off-by: Shiyang Ruan <ruansy.fnst@xxxxxxxxxxx>
> > ---
> >   fs/xfs/xfs_notify_failure.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/xfs_notify_failure.c b/fs/xfs/xfs_notify_failure.c
> > index 4a9bbd3fe120..459fc8a39635 100644
> > --- a/fs/xfs/xfs_notify_failure.c
> > +++ b/fs/xfs/xfs_notify_failure.c
> > @@ -151,7 +151,7 @@ xfs_dax_notify_ddev_failure(
> >   		agend = min(be32_to_cpu(agf->agf_length),
> >   				ri_high.rm_startblock);

I don't understand this.  ri_high.rm_startblock should be the last agbno
for which we want rmapbt mappings.  If agf_length is 100, then don't we
want to be clamping agend to 99, not 100?  Block 99 is the last block in
an AG.

	agend = min(be32_to_cpu(agf->agf_length) - 1,
		    ri_high.rm_startblock);

If we do the above...

> >   		notify.startblock = ri_low.rm_startblock;
> > -		notify.blockcount = agend - ri_low.rm_startblock;
> > +		notify.blockcount = agend + 1 - ri_low.rm_startblock;

...then this actually makes sense.

> >   		error = xfs_rmap_query_range(cur, &ri_low, &ri_high,
> >   				xfs_dax_failure_fn, &notify);

Sorry I've been kinda slow to respond.

--D