Re: [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary()

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

On Wed, Jan 28, 2026 at 11:25:17PM +0800, Alan Huang wrote:
> On Jan 28, 2026, at 23:14, Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com> wrote:
> > 
> > We should ASSERT on a variable before using it, so that we
> > don't end up using an illegal value.
> > 
> > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> > ---
> > fs/xfs/xfs_rtalloc.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> > index a12ffed12391..9fb975171bf8 100644
> > --- a/fs/xfs/xfs_rtalloc.c
> > +++ b/fs/xfs/xfs_rtalloc.c
> > @@ -112,6 +112,11 @@ xfs_rtcopy_summary(
> > error = xfs_rtget_summary(oargs, log, bbno, &sum);
> > if (error)
> > goto out;
> > + if (sum < 0) {
> > + ASSERT(sum >= 0);
> 
> 
> Does the ASSERT make sense under the if condition ?

Yes, it's logging that the function failed due to some sort of loop
control problem, rather than the current behavior where it logs the
assert failure and .... keeps going.

--D

> 
> > + error = -EFSCORRUPTED;
> > + goto out;
> > + }
> > if (sum == 0)
> > continue;
> > error = xfs_rtmodify_summary(oargs, log, bbno, -sum);
> > @@ -120,7 +125,6 @@ xfs_rtcopy_summary(
> > error = xfs_rtmodify_summary(nargs, log, bbno, sum);
> > if (error)
> > goto out;
> > - ASSERT(sum > 0);
> > }
> > }
> > error = 0;
> > -- 
> > 2.43.5
> > 
> > 
> 
>