public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] Misc fixes in XFS realtime
@ 2026-01-28 15:14 Nirjhar Roy (IBM)
  2026-01-28 15:14 ` [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary() Nirjhar Roy (IBM)
  2026-01-28 15:14 ` [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range() Nirjhar Roy (IBM)
  0 siblings, 2 replies; 12+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-01-28 15:14 UTC (permalink / raw)
  To: linux-xfs; +Cc: ritesh.list, ojaswin, djwong, hch, nirjhar.roy.lists

This patchset has 2 fixes in some XFS realtime code. Details are
in the commit messages.

PLEASE NOTE that patch 1/2 in this patchset is actually a V2 of [1].

[1] https://lore.kernel.org/all/20260122181148.GE5945@frogsfrogsfrogs/

Nirjhar Roy (IBM) (2):
  xfs: Move ASSERTion location in xfs_rtcopy_summary()
  xfs: Fix in xfs_rtalloc_query_range()

 fs/xfs/libxfs/xfs_rtbitmap.c | 2 +-
 fs/xfs/xfs_rtalloc.c         | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.43.5


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary()
  2026-01-28 15:14 [PATCH v1 0/2] Misc fixes in XFS realtime Nirjhar Roy (IBM)
@ 2026-01-28 15:14 ` Nirjhar Roy (IBM)
  2026-01-28 15:25   ` Alan Huang
  2026-01-28 16:13   ` Darrick J. Wong
  2026-01-28 15:14 ` [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range() Nirjhar Roy (IBM)
  1 sibling, 2 replies; 12+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-01-28 15:14 UTC (permalink / raw)
  To: linux-xfs; +Cc: ritesh.list, ojaswin, djwong, hch, nirjhar.roy.lists

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);
+				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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range()
  2026-01-28 15:14 [PATCH v1 0/2] Misc fixes in XFS realtime Nirjhar Roy (IBM)
  2026-01-28 15:14 ` [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary() Nirjhar Roy (IBM)
@ 2026-01-28 15:14 ` Nirjhar Roy (IBM)
  2026-01-28 17:14   ` Darrick J. Wong
  1 sibling, 1 reply; 12+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-01-28 15:14 UTC (permalink / raw)
  To: linux-xfs; +Cc: ritesh.list, ojaswin, djwong, hch, nirjhar.roy.lists

xfs_rtalloc_query_range() should not return 0 by doing a NOP when
start == end i.e, when the rtgroup size is 1. This causes incorrect
calculation of free rtextents i.e, the count is reduced by 1 since
the last rtgroup's rtextent count is not taken and hence xfs_scrub
throws false summary counter report (from xchk_fscounters()).

A simple way to reproduce the above bug:

$ mkfs.xfs -f -m metadir=1 \
	-r rtdev=/dev/loop2,extsize=4096,rgcount=4,size=1G \
	-d size=1G /dev/loop1
meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=1, rmapbt=1
         =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
         =                       exchange=1   metadir=1
data     =                       bsize=4096   blocks=262144, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
log      =internal log           bsize=4096   blocks=16384, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
         =                       rgcount=4    rgsize=65536 extents
         =                       zoned=0      start=0 reserved=0
Discarding blocks...Done.
Discarding blocks...Done.
$ mount -o rtdev=/dev/loop2 /dev/loop1 /mnt1/scratch
$ xfs_growfs -R $(( 65536 * 4 + 1 ))  /mnt1/scratch
meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=1, rmapbt=1
         =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
         =                       exchange=1   metadir=1
data     =                       bsize=4096   blocks=262144, imaxpct=25
         =                       sunit=0      swidth=0 blks
naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
log      =internal log           bsize=4096   blocks=16384, version=2
         =                       sectsz=512   sunit=0 blks, lazy-count=1
realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
         =                       rgcount=4    rgsize=65536 extents
         =                       zoned=0      start=0 reserved=0
calling xfsctl with in.newblocks = 262145
realtime blocks changed from 262144 to 262145
$ xfs_scrub -n   -v /mnt1/scratch
Phase 1: Find filesystem geometry.
/mnt1/scratch: using 2 threads to scrub.
Phase 2: Check internal metadata.
Corruption: rtgroup 4 realtime summary: Repairs are required.
Phase 3: Scan all inodes.
Phase 5: Check directory tree.
Info: /mnt1/scratch: Filesystem has errors, skipping connectivity checks.
Phase 7: Check summary counters.
Corruption: filesystem summary counters: Repairs are required.
125.0MiB data used;  8.0KiB realtime data used;  15 inodes used.
64.3MiB data found; 4.0KiB realtime data found; 18 inodes found.
18 inodes counted; 18 inodes checked.
Phase 8: Trim filesystem storage.
/mnt1/scratch: corruptions found: 2
/mnt1/scratch: Re-run xfs_scrub without -n.

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
 fs/xfs/libxfs/xfs_rtbitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
index 618061d898d4..8f552129ffcc 100644
--- a/fs/xfs/libxfs/xfs_rtbitmap.c
+++ b/fs/xfs/libxfs/xfs_rtbitmap.c
@@ -1170,7 +1170,7 @@ xfs_rtalloc_query_range(
 
 	if (start > end)
 		return -EINVAL;
-	if (start == end || start >= rtg->rtg_extents)
+	if (start >= rtg->rtg_extents)
 		return 0;
 
 	end = min(end, rtg->rtg_extents - 1);
-- 
2.43.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary()
  2026-01-28 15:14 ` [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary() Nirjhar Roy (IBM)
@ 2026-01-28 15:25   ` Alan Huang
  2026-01-28 16:14     ` Darrick J. Wong
  2026-01-28 16:13   ` Darrick J. Wong
  1 sibling, 1 reply; 12+ messages in thread
From: Alan Huang @ 2026-01-28 15:25 UTC (permalink / raw)
  To: Nirjhar Roy (IBM); +Cc: linux-xfs, ritesh.list, ojaswin, djwong, hch

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 ?


> + 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
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary()
  2026-01-28 15:14 ` [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary() Nirjhar Roy (IBM)
  2026-01-28 15:25   ` Alan Huang
@ 2026-01-28 16:13   ` Darrick J. Wong
  2026-01-28 17:00     ` Nirjhar Roy (IBM)
  1 sibling, 1 reply; 12+ messages in thread
From: Darrick J. Wong @ 2026-01-28 16:13 UTC (permalink / raw)
  To: Nirjhar Roy (IBM); +Cc: linux-xfs, ritesh.list, ojaswin, hch

On Wed, Jan 28, 2026 at 08:44:34PM +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..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);
> +				error = -EFSCORRUPTED;
> +				goto out;

Thanks for making this a bailout case,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> +			}
>  			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
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary()
  2026-01-28 15:25   ` Alan Huang
@ 2026-01-28 16:14     ` Darrick J. Wong
  0 siblings, 0 replies; 12+ messages in thread
From: Darrick J. Wong @ 2026-01-28 16:14 UTC (permalink / raw)
  To: Alan Huang; +Cc: Nirjhar Roy (IBM), linux-xfs, ritesh.list, ojaswin, hch

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
> > 
> > 
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary()
  2026-01-28 16:13   ` Darrick J. Wong
@ 2026-01-28 17:00     ` Nirjhar Roy (IBM)
  0 siblings, 0 replies; 12+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-01-28 17:00 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, ritesh.list, ojaswin, hch, Carlos Maiolino


On 1/28/26 21:43, Darrick J. Wong wrote:
> On Wed, Jan 28, 2026 at 08:44:34PM +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..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);
>> +				error = -EFSCORRUPTED;
>> +				goto out;
> Thanks for making this a bailout case,
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Thank you for your suggestion.

--NR

>
> --D
>
>> +			}
>>   			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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range()
  2026-01-28 15:14 ` [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range() Nirjhar Roy (IBM)
@ 2026-01-28 17:14   ` Darrick J. Wong
  2026-01-28 18:00     ` Nirjhar Roy (IBM)
  2026-02-02  6:25     ` Christoph Hellwig
  0 siblings, 2 replies; 12+ messages in thread
From: Darrick J. Wong @ 2026-01-28 17:14 UTC (permalink / raw)
  To: Nirjhar Roy (IBM); +Cc: linux-xfs, ritesh.list, ojaswin, hch

On Wed, Jan 28, 2026 at 08:44:35PM +0530, Nirjhar Roy (IBM) wrote:
> xfs_rtalloc_query_range() should not return 0 by doing a NOP when
> start == end i.e, when the rtgroup size is 1. This causes incorrect
> calculation of free rtextents i.e, the count is reduced by 1 since
> the last rtgroup's rtextent count is not taken and hence xfs_scrub
> throws false summary counter report (from xchk_fscounters()).
> 
> A simple way to reproduce the above bug:
> 
> $ mkfs.xfs -f -m metadir=1 \
> 	-r rtdev=/dev/loop2,extsize=4096,rgcount=4,size=1G \
> 	-d size=1G /dev/loop1
> meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
>          =                       sectsz=512   attr=2, projid32bit=1
>          =                       crc=1        finobt=1, sparse=1, rmapbt=1
>          =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
>          =                       exchange=1   metadir=1
> data     =                       bsize=4096   blocks=262144, imaxpct=25
>          =                       sunit=0      swidth=0 blks
> naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
> log      =internal log           bsize=4096   blocks=16384, version=2
>          =                       sectsz=512   sunit=0 blks, lazy-count=1
> realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
>          =                       rgcount=4    rgsize=65536 extents
>          =                       zoned=0      start=0 reserved=0
> Discarding blocks...Done.
> Discarding blocks...Done.
> $ mount -o rtdev=/dev/loop2 /dev/loop1 /mnt1/scratch
> $ xfs_growfs -R $(( 65536 * 4 + 1 ))  /mnt1/scratch
> meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
>          =                       sectsz=512   attr=2, projid32bit=1
>          =                       crc=1        finobt=1, sparse=1, rmapbt=1
>          =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
>          =                       exchange=1   metadir=1
> data     =                       bsize=4096   blocks=262144, imaxpct=25
>          =                       sunit=0      swidth=0 blks
> naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
> log      =internal log           bsize=4096   blocks=16384, version=2
>          =                       sectsz=512   sunit=0 blks, lazy-count=1
> realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
>          =                       rgcount=4    rgsize=65536 extents
>          =                       zoned=0      start=0 reserved=0
> calling xfsctl with in.newblocks = 262145
> realtime blocks changed from 262144 to 262145
> $ xfs_scrub -n   -v /mnt1/scratch
> Phase 1: Find filesystem geometry.
> /mnt1/scratch: using 2 threads to scrub.
> Phase 2: Check internal metadata.
> Corruption: rtgroup 4 realtime summary: Repairs are required.
> Phase 3: Scan all inodes.
> Phase 5: Check directory tree.
> Info: /mnt1/scratch: Filesystem has errors, skipping connectivity checks.
> Phase 7: Check summary counters.
> Corruption: filesystem summary counters: Repairs are required.
> 125.0MiB data used;  8.0KiB realtime data used;  15 inodes used.
> 64.3MiB data found; 4.0KiB realtime data found; 18 inodes found.
> 18 inodes counted; 18 inodes checked.
> Phase 8: Trim filesystem storage.
> /mnt1/scratch: corruptions found: 2
> /mnt1/scratch: Re-run xfs_scrub without -n.
> 
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>

Yeah, that looks right.  I can also reproduce it with:

# mkfs.xfs -m metadir=1 -r rtdev=/dev/sdb /dev/sda -r rgsize=65536b,size=131073b -f
# mount /dev/sda /mnt -o rtdev=/dev/sdb
# xfs_scrub -dTvn /mnt

Cc: <stable@vger.kernel.org> # v6.13
Fixes: e3088ae2dcae3c ("xfs: move RT bitmap and summary information to the rtgroup")

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D
> ---
>  fs/xfs/libxfs/xfs_rtbitmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> index 618061d898d4..8f552129ffcc 100644
> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> @@ -1170,7 +1170,7 @@ xfs_rtalloc_query_range(
>  
>  	if (start > end)
>  		return -EINVAL;
> -	if (start == end || start >= rtg->rtg_extents)
> +	if (start >= rtg->rtg_extents)
>  		return 0;
>  
>  	end = min(end, rtg->rtg_extents - 1);
> -- 
> 2.43.5
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range()
  2026-01-28 17:14   ` Darrick J. Wong
@ 2026-01-28 18:00     ` Nirjhar Roy (IBM)
  2026-01-28 18:29       ` Darrick J. Wong
  2026-02-02  6:25     ` Christoph Hellwig
  1 sibling, 1 reply; 12+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-01-28 18:00 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, ritesh.list, ojaswin, hch


On 1/28/26 22:44, Darrick J. Wong wrote:
> On Wed, Jan 28, 2026 at 08:44:35PM +0530, Nirjhar Roy (IBM) wrote:
>> xfs_rtalloc_query_range() should not return 0 by doing a NOP when
>> start == end i.e, when the rtgroup size is 1. This causes incorrect
>> calculation of free rtextents i.e, the count is reduced by 1 since
>> the last rtgroup's rtextent count is not taken and hence xfs_scrub
>> throws false summary counter report (from xchk_fscounters()).
>>
>> A simple way to reproduce the above bug:
>>
>> $ mkfs.xfs -f -m metadir=1 \
>> 	-r rtdev=/dev/loop2,extsize=4096,rgcount=4,size=1G \
>> 	-d size=1G /dev/loop1
>> meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
>>           =                       sectsz=512   attr=2, projid32bit=1
>>           =                       crc=1        finobt=1, sparse=1, rmapbt=1
>>           =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
>>           =                       exchange=1   metadir=1
>> data     =                       bsize=4096   blocks=262144, imaxpct=25
>>           =                       sunit=0      swidth=0 blks
>> naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
>> log      =internal log           bsize=4096   blocks=16384, version=2
>>           =                       sectsz=512   sunit=0 blks, lazy-count=1
>> realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
>>           =                       rgcount=4    rgsize=65536 extents
>>           =                       zoned=0      start=0 reserved=0
>> Discarding blocks...Done.
>> Discarding blocks...Done.
>> $ mount -o rtdev=/dev/loop2 /dev/loop1 /mnt1/scratch
>> $ xfs_growfs -R $(( 65536 * 4 + 1 ))  /mnt1/scratch
>> meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
>>           =                       sectsz=512   attr=2, projid32bit=1
>>           =                       crc=1        finobt=1, sparse=1, rmapbt=1
>>           =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
>>           =                       exchange=1   metadir=1
>> data     =                       bsize=4096   blocks=262144, imaxpct=25
>>           =                       sunit=0      swidth=0 blks
>> naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
>> log      =internal log           bsize=4096   blocks=16384, version=2
>>           =                       sectsz=512   sunit=0 blks, lazy-count=1
>> realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
>>           =                       rgcount=4    rgsize=65536 extents
>>           =                       zoned=0      start=0 reserved=0
>> calling xfsctl with in.newblocks = 262145
>> realtime blocks changed from 262144 to 262145
>> $ xfs_scrub -n   -v /mnt1/scratch
>> Phase 1: Find filesystem geometry.
>> /mnt1/scratch: using 2 threads to scrub.
>> Phase 2: Check internal metadata.
>> Corruption: rtgroup 4 realtime summary: Repairs are required.
>> Phase 3: Scan all inodes.
>> Phase 5: Check directory tree.
>> Info: /mnt1/scratch: Filesystem has errors, skipping connectivity checks.
>> Phase 7: Check summary counters.
>> Corruption: filesystem summary counters: Repairs are required.
>> 125.0MiB data used;  8.0KiB realtime data used;  15 inodes used.
>> 64.3MiB data found; 4.0KiB realtime data found; 18 inodes found.
>> 18 inodes counted; 18 inodes checked.
>> Phase 8: Trim filesystem storage.
>> /mnt1/scratch: corruptions found: 2
>> /mnt1/scratch: Re-run xfs_scrub without -n.
>>
>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> Yeah, that looks right.  I can also reproduce it with:
>
> # mkfs.xfs -m metadir=1 -r rtdev=/dev/sdb /dev/sda -r rgsize=65536b,size=131073b -f
> # mount /dev/sda /mnt -o rtdev=/dev/sdb
> # xfs_scrub -dTvn /mnt
>
> Cc: <stable@vger.kernel.org> # v6.13
> Fixes: e3088ae2dcae3c ("xfs: move RT bitmap and summary information to the rtgroup")
>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

Thank you. Will the "Cc" and "Fixes" tag be added from this 
automatically, or do I need to re-send it by updating the commit message?

--NR

>
> --D
>> ---
>>   fs/xfs/libxfs/xfs_rtbitmap.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
>> index 618061d898d4..8f552129ffcc 100644
>> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
>> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
>> @@ -1170,7 +1170,7 @@ xfs_rtalloc_query_range(
>>   
>>   	if (start > end)
>>   		return -EINVAL;
>> -	if (start == end || start >= rtg->rtg_extents)
>> +	if (start >= rtg->rtg_extents)
>>   		return 0;
>>   
>>   	end = min(end, rtg->rtg_extents - 1);
>> -- 
>> 2.43.5
>>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range()
  2026-01-28 18:00     ` Nirjhar Roy (IBM)
@ 2026-01-28 18:29       ` Darrick J. Wong
  2026-01-28 18:30         ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 12+ messages in thread
From: Darrick J. Wong @ 2026-01-28 18:29 UTC (permalink / raw)
  To: Nirjhar Roy (IBM); +Cc: linux-xfs, ritesh.list, ojaswin, hch

On Wed, Jan 28, 2026 at 11:30:18PM +0530, Nirjhar Roy (IBM) wrote:
> 
> On 1/28/26 22:44, Darrick J. Wong wrote:
> > On Wed, Jan 28, 2026 at 08:44:35PM +0530, Nirjhar Roy (IBM) wrote:
> > > xfs_rtalloc_query_range() should not return 0 by doing a NOP when
> > > start == end i.e, when the rtgroup size is 1. This causes incorrect
> > > calculation of free rtextents i.e, the count is reduced by 1 since
> > > the last rtgroup's rtextent count is not taken and hence xfs_scrub
> > > throws false summary counter report (from xchk_fscounters()).
> > > 
> > > A simple way to reproduce the above bug:
> > > 
> > > $ mkfs.xfs -f -m metadir=1 \
> > > 	-r rtdev=/dev/loop2,extsize=4096,rgcount=4,size=1G \
> > > 	-d size=1G /dev/loop1
> > > meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
> > >           =                       sectsz=512   attr=2, projid32bit=1
> > >           =                       crc=1        finobt=1, sparse=1, rmapbt=1
> > >           =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
> > >           =                       exchange=1   metadir=1
> > > data     =                       bsize=4096   blocks=262144, imaxpct=25
> > >           =                       sunit=0      swidth=0 blks
> > > naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
> > > log      =internal log           bsize=4096   blocks=16384, version=2
> > >           =                       sectsz=512   sunit=0 blks, lazy-count=1
> > > realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
> > >           =                       rgcount=4    rgsize=65536 extents
> > >           =                       zoned=0      start=0 reserved=0
> > > Discarding blocks...Done.
> > > Discarding blocks...Done.
> > > $ mount -o rtdev=/dev/loop2 /dev/loop1 /mnt1/scratch
> > > $ xfs_growfs -R $(( 65536 * 4 + 1 ))  /mnt1/scratch
> > > meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
> > >           =                       sectsz=512   attr=2, projid32bit=1
> > >           =                       crc=1        finobt=1, sparse=1, rmapbt=1
> > >           =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
> > >           =                       exchange=1   metadir=1
> > > data     =                       bsize=4096   blocks=262144, imaxpct=25
> > >           =                       sunit=0      swidth=0 blks
> > > naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
> > > log      =internal log           bsize=4096   blocks=16384, version=2
> > >           =                       sectsz=512   sunit=0 blks, lazy-count=1
> > > realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
> > >           =                       rgcount=4    rgsize=65536 extents
> > >           =                       zoned=0      start=0 reserved=0
> > > calling xfsctl with in.newblocks = 262145
> > > realtime blocks changed from 262144 to 262145
> > > $ xfs_scrub -n   -v /mnt1/scratch
> > > Phase 1: Find filesystem geometry.
> > > /mnt1/scratch: using 2 threads to scrub.
> > > Phase 2: Check internal metadata.
> > > Corruption: rtgroup 4 realtime summary: Repairs are required.
> > > Phase 3: Scan all inodes.
> > > Phase 5: Check directory tree.
> > > Info: /mnt1/scratch: Filesystem has errors, skipping connectivity checks.
> > > Phase 7: Check summary counters.
> > > Corruption: filesystem summary counters: Repairs are required.
> > > 125.0MiB data used;  8.0KiB realtime data used;  15 inodes used.
> > > 64.3MiB data found; 4.0KiB realtime data found; 18 inodes found.
> > > 18 inodes counted; 18 inodes checked.
> > > Phase 8: Trim filesystem storage.
> > > /mnt1/scratch: corruptions found: 2
> > > /mnt1/scratch: Re-run xfs_scrub without -n.
> > > 
> > > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> > Yeah, that looks right.  I can also reproduce it with:
> > 
> > # mkfs.xfs -m metadir=1 -r rtdev=/dev/sdb /dev/sda -r rgsize=65536b,size=131073b -f
> > # mount /dev/sda /mnt -o rtdev=/dev/sdb
> > # xfs_scrub -dTvn /mnt
> > 
> > Cc: <stable@vger.kernel.org> # v6.13
> > Fixes: e3088ae2dcae3c ("xfs: move RT bitmap and summary information to the rtgroup")
> > 
> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> Thank you. Will the "Cc" and "Fixes" tag be added from this automatically,
> or do I need to re-send it by updating the commit message?

cem might add it on his own, but you could also resubmit with all three
trailers added, to keep things simple for him.

--D

> --NR
> 
> > 
> > --D
> > > ---
> > >   fs/xfs/libxfs/xfs_rtbitmap.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
> > > index 618061d898d4..8f552129ffcc 100644
> > > --- a/fs/xfs/libxfs/xfs_rtbitmap.c
> > > +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
> > > @@ -1170,7 +1170,7 @@ xfs_rtalloc_query_range(
> > >   	if (start > end)
> > >   		return -EINVAL;
> > > -	if (start == end || start >= rtg->rtg_extents)
> > > +	if (start >= rtg->rtg_extents)
> > >   		return 0;
> > >   	end = min(end, rtg->rtg_extents - 1);
> > > -- 
> > > 2.43.5
> > > 
> -- 
> Nirjhar Roy
> Linux Kernel Developer
> IBM, Bangalore
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range()
  2026-01-28 18:29       ` Darrick J. Wong
@ 2026-01-28 18:30         ` Nirjhar Roy (IBM)
  0 siblings, 0 replies; 12+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-01-28 18:30 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: linux-xfs, ritesh.list, ojaswin, hch


On 1/28/26 23:59, Darrick J. Wong wrote:
> On Wed, Jan 28, 2026 at 11:30:18PM +0530, Nirjhar Roy (IBM) wrote:
>> On 1/28/26 22:44, Darrick J. Wong wrote:
>>> On Wed, Jan 28, 2026 at 08:44:35PM +0530, Nirjhar Roy (IBM) wrote:
>>>> xfs_rtalloc_query_range() should not return 0 by doing a NOP when
>>>> start == end i.e, when the rtgroup size is 1. This causes incorrect
>>>> calculation of free rtextents i.e, the count is reduced by 1 since
>>>> the last rtgroup's rtextent count is not taken and hence xfs_scrub
>>>> throws false summary counter report (from xchk_fscounters()).
>>>>
>>>> A simple way to reproduce the above bug:
>>>>
>>>> $ mkfs.xfs -f -m metadir=1 \
>>>> 	-r rtdev=/dev/loop2,extsize=4096,rgcount=4,size=1G \
>>>> 	-d size=1G /dev/loop1
>>>> meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
>>>>            =                       sectsz=512   attr=2, projid32bit=1
>>>>            =                       crc=1        finobt=1, sparse=1, rmapbt=1
>>>>            =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
>>>>            =                       exchange=1   metadir=1
>>>> data     =                       bsize=4096   blocks=262144, imaxpct=25
>>>>            =                       sunit=0      swidth=0 blks
>>>> naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
>>>> log      =internal log           bsize=4096   blocks=16384, version=2
>>>>            =                       sectsz=512   sunit=0 blks, lazy-count=1
>>>> realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
>>>>            =                       rgcount=4    rgsize=65536 extents
>>>>            =                       zoned=0      start=0 reserved=0
>>>> Discarding blocks...Done.
>>>> Discarding blocks...Done.
>>>> $ mount -o rtdev=/dev/loop2 /dev/loop1 /mnt1/scratch
>>>> $ xfs_growfs -R $(( 65536 * 4 + 1 ))  /mnt1/scratch
>>>> meta-data=/dev/loop1             isize=512    agcount=4, agsize=65536 blks
>>>>            =                       sectsz=512   attr=2, projid32bit=1
>>>>            =                       crc=1        finobt=1, sparse=1, rmapbt=1
>>>>            =                       reflink=0    bigtime=1 inobtcount=1 nrext64=1
>>>>            =                       exchange=1   metadir=1
>>>> data     =                       bsize=4096   blocks=262144, imaxpct=25
>>>>            =                       sunit=0      swidth=0 blks
>>>> naming   =version 2              bsize=4096   ascii-ci=0, ftype=1, parent=0
>>>> log      =internal log           bsize=4096   blocks=16384, version=2
>>>>            =                       sectsz=512   sunit=0 blks, lazy-count=1
>>>> realtime =/dev/loop2             extsz=4096   blocks=262144, rtextents=262144
>>>>            =                       rgcount=4    rgsize=65536 extents
>>>>            =                       zoned=0      start=0 reserved=0
>>>> calling xfsctl with in.newblocks = 262145
>>>> realtime blocks changed from 262144 to 262145
>>>> $ xfs_scrub -n   -v /mnt1/scratch
>>>> Phase 1: Find filesystem geometry.
>>>> /mnt1/scratch: using 2 threads to scrub.
>>>> Phase 2: Check internal metadata.
>>>> Corruption: rtgroup 4 realtime summary: Repairs are required.
>>>> Phase 3: Scan all inodes.
>>>> Phase 5: Check directory tree.
>>>> Info: /mnt1/scratch: Filesystem has errors, skipping connectivity checks.
>>>> Phase 7: Check summary counters.
>>>> Corruption: filesystem summary counters: Repairs are required.
>>>> 125.0MiB data used;  8.0KiB realtime data used;  15 inodes used.
>>>> 64.3MiB data found; 4.0KiB realtime data found; 18 inodes found.
>>>> 18 inodes counted; 18 inodes checked.
>>>> Phase 8: Trim filesystem storage.
>>>> /mnt1/scratch: corruptions found: 2
>>>> /mnt1/scratch: Re-run xfs_scrub without -n.
>>>>
>>>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>>> Yeah, that looks right.  I can also reproduce it with:
>>>
>>> # mkfs.xfs -m metadir=1 -r rtdev=/dev/sdb /dev/sda -r rgsize=65536b,size=131073b -f
>>> # mount /dev/sda /mnt -o rtdev=/dev/sdb
>>> # xfs_scrub -dTvn /mnt
>>>
>>> Cc: <stable@vger.kernel.org> # v6.13
>>> Fixes: e3088ae2dcae3c ("xfs: move RT bitmap and summary information to the rtgroup")
>>>
>>> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
>> Thank you. Will the "Cc" and "Fixes" tag be added from this automatically,
>> or do I need to re-send it by updating the commit message?
> cem might add it on his own, but you could also resubmit with all three
> trailers added, to keep things simple for him.

Sure, I can do that. Thank you.

--NR

>
> --D
>
>> --NR
>>
>>> --D
>>>> ---
>>>>    fs/xfs/libxfs/xfs_rtbitmap.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/xfs/libxfs/xfs_rtbitmap.c b/fs/xfs/libxfs/xfs_rtbitmap.c
>>>> index 618061d898d4..8f552129ffcc 100644
>>>> --- a/fs/xfs/libxfs/xfs_rtbitmap.c
>>>> +++ b/fs/xfs/libxfs/xfs_rtbitmap.c
>>>> @@ -1170,7 +1170,7 @@ xfs_rtalloc_query_range(
>>>>    	if (start > end)
>>>>    		return -EINVAL;
>>>> -	if (start == end || start >= rtg->rtg_extents)
>>>> +	if (start >= rtg->rtg_extents)
>>>>    		return 0;
>>>>    	end = min(end, rtg->rtg_extents - 1);
>>>> -- 
>>>> 2.43.5
>>>>
>> -- 
>> Nirjhar Roy
>> Linux Kernel Developer
>> IBM, Bangalore
>>
>>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range()
  2026-01-28 17:14   ` Darrick J. Wong
  2026-01-28 18:00     ` Nirjhar Roy (IBM)
@ 2026-02-02  6:25     ` Christoph Hellwig
  1 sibling, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2026-02-02  6:25 UTC (permalink / raw)
  To: Darrick J. Wong; +Cc: Nirjhar Roy (IBM), linux-xfs, ritesh.list, ojaswin, hch

On Wed, Jan 28, 2026 at 09:14:43AM -0800, Darrick J. Wong wrote:
> # mkfs.xfs -m metadir=1 -r rtdev=/dev/sdb /dev/sda -r rgsize=65536b,size=131073b -f
> # mount /dev/sda /mnt -o rtdev=/dev/sdb
> # xfs_scrub -dTvn /mnt

Can we get this wired up in xfstests?


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-02-02  6:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-28 15:14 [PATCH v1 0/2] Misc fixes in XFS realtime Nirjhar Roy (IBM)
2026-01-28 15:14 ` [PATCH v1 1/2] xfs: Move ASSERTion location in xfs_rtcopy_summary() Nirjhar Roy (IBM)
2026-01-28 15:25   ` Alan Huang
2026-01-28 16:14     ` Darrick J. Wong
2026-01-28 16:13   ` Darrick J. Wong
2026-01-28 17:00     ` Nirjhar Roy (IBM)
2026-01-28 15:14 ` [PATCH v1 2/2] xfs: Fix in xfs_rtalloc_query_range() Nirjhar Roy (IBM)
2026-01-28 17:14   ` Darrick J. Wong
2026-01-28 18:00     ` Nirjhar Roy (IBM)
2026-01-28 18:29       ` Darrick J. Wong
2026-01-28 18:30         ` Nirjhar Roy (IBM)
2026-02-02  6:25     ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox