Re: [patch v3 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary()

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 2/3/26 21:03, Darrick J. Wong wrote:
> On Tue, Feb 03, 2026 at 08:24:28PM +0530, Nirjhar Roy (IBM) 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..727582b98b27 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) {
> Oh, heh.  I never replied to your question about XFS_IS_CORRUPT.
> That's the macro helper to report metadata corruptions that aren't
> caught by the verifiers (e.g. you expected a nonzero summary counter,
> but it was zero) because verifiers only look for discrepancies within a
> block.
Okay, thank you for the explanation.
>
> IOWs, this would be fine:
>
> 			if (XFS_IS_CORRUPT(oargs->mp, sum < 0)) {
> 				error = -EFSCORRUPTED;
> 				goto out;
> 			}
>
> --D

Okay, thank you. I can make the change.

--NR

>
>> +				ASSERT(0);
>> +				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
>>
>>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore