public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/4] xfs: Misc changes to XFS realtime
@ 2026-02-12 14:01 Nirjhar Roy (IBM)
  2026-02-12 14:01 ` [PATCH v1 1/4] xfs: Fix xfs_last_rt_bmblock() Nirjhar Roy (IBM)
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-12 14:01 UTC (permalink / raw)
  To: djwong, hch, cem; +Cc: linux-xfs, ritesh.list, ojaswin, nirjhar.roy.lists

This series has a bug fix and adds some missing operations to
growfs code in the realtime code. Details are in the commit messages

Nirjhar Roy (IBM) (4):
  xfs: Fix xfs_last_rt_bmblock()
  xfs: Update sb_frextents when lazy count is set
  xfs: Update lazy counters in xfs_growfs_rt_bmblock()
  xfs: Add some comments in some macros.

 fs/xfs/libxfs/xfs_sb.c | 23 ++++++++++++-----------
 fs/xfs/xfs_linux.h     |  6 ++++++
 fs/xfs/xfs_rtalloc.c   | 33 +++++++++++++++++++++++++++------
 3 files changed, 45 insertions(+), 17 deletions(-)

--
2.43.5


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

* [PATCH v1 1/4] xfs: Fix xfs_last_rt_bmblock()
  2026-02-12 14:01 [PATCH v1 0/4] xfs: Misc changes to XFS realtime Nirjhar Roy (IBM)
@ 2026-02-12 14:01 ` Nirjhar Roy (IBM)
  2026-02-18  5:53   ` Christoph Hellwig
  2026-02-12 14:01 ` [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set Nirjhar Roy (IBM)
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-12 14:01 UTC (permalink / raw)
  To: djwong, hch, cem; +Cc: linux-xfs, ritesh.list, ojaswin, nirjhar.roy.lists

Bug description:

If the size of the last rtgroup i.e, the rtg passed to
xfs_last_rt_bmblock() is such that the last rtextent falls in 0th word
offset of a bmblock of the bitmap file tracking this (last) rtgroup,
then in that case xfs_last_rt_bmblock() incorrectly returns the next
bmblock number instead of the current/last used bmblock number.
When xfs_last_rt_bmblock() incorrectly returns the next bmblock,
the loop to grow/modify the bmblocks in xfs_growfs_rtg() doesn't
execute and xfs_growfs basically does a nop in certain cases.

xfs_growfs will do a nop when the new size of the fs will have the same
number of rtgroups i.e, we are only growing the last rtgroup.

Reproduce:
$ mkfs.xfs -m metadir=0 -r rtdev=/dev/loop1 /dev/loop0 \
	-r rgsize=32768b,size=32769b -f
$ mount -o rtdev=/dev/loop1 /dev/loop0 /mnt/scratch
$ xfs_growfs -R $(( 32769 + 1 )) /mnt/scratch
$ xfs_info /mnt/scratch | grep rtextents
$ # We can see that rtextents hasn't changed

Fix:
Fix this by returning the current/last used bmblock when the last
rtgroup size is not a multiple xfs_rtbitmap_rtx_per_rbmblock()
and the next bmblock when the rtgroup size is a multiple of
xfs_rtbitmap_rtx_per_rbmblock() i.e, the existing blocks are
completely used up.
Also, I have renamed xfs_last_rt_bmblock() to
xfs_last_rt_bmblock_to_extend() to signify that this function
returns the bmblock number to extend and NOT always the last used
bmblock number.

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
 fs/xfs/xfs_rtalloc.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index a12ffed12391..a7a0859ed47f 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -1079,17 +1079,27 @@ xfs_last_rtgroup_extents(
 }
 
 /*
- * Calculate the last rbmblock currently used.
+ * This will return the bitmap block number (indexed at 0) that will be
+ * extended/modified. There are 2 cases here:
+ * 1. The size of the rtg is such that it is a multiple of
+ *    xfs_rtbitmap_rtx_per_rbmblock() i.e, an integral number of bitmap blocks
+ *    are completely filled up. In this case, we should return
+ *    1 + (the last used bitmap block number).
+ * 2. The size of the rtg is not an multiple of xfs_rtbitmap_rtx_per_rbmblock().
+ *    Here we will return the block number of last used block number. In this
+ *    case, we will modify the last used bitmap block to extend the size of the
+ *    rtgroup.
  *
  * This also deals with the case where there were no rtextents before.
  */
 static xfs_fileoff_t
-xfs_last_rt_bmblock(
+xfs_last_rt_bmblock_to_extend(
 	struct xfs_rtgroup	*rtg)
 {
 	struct xfs_mount	*mp = rtg_mount(rtg);
 	xfs_rgnumber_t		rgno = rtg_rgno(rtg);
 	xfs_fileoff_t		bmbno = 0;
+	unsigned int		mod = 0;
 
 	ASSERT(!mp->m_sb.sb_rgcount || rgno >= mp->m_sb.sb_rgcount - 1);
 
@@ -1097,9 +1107,16 @@ xfs_last_rt_bmblock(
 		xfs_rtxnum_t	nrext = xfs_last_rtgroup_extents(mp);
 
 		/* Also fill up the previous block if not entirely full. */
-		bmbno = xfs_rtbitmap_blockcount_len(mp, nrext);
-		if (xfs_rtx_to_rbmword(mp, nrext) != 0)
-			bmbno--;
+		/* We are doing a -1 to convert it to a 0 based index */
+		bmbno = xfs_rtbitmap_blockcount_len(mp, nrext) - 1;
+		div_u64_rem(nrext, xfs_rtbitmap_rtx_per_rbmblock(mp), &mod);
+		/*
+		 * mod = 0 means that all the current blocks are full. So
+		 * return the next block number to be used for the rtgroup
+		 * growth.
+		 */
+		if (mod == 0)
+			bmbno++;
 	}
 
 	return bmbno;
@@ -1204,7 +1221,8 @@ xfs_growfs_rtg(
 			goto out_rele;
 	}
 
-	for (bmbno = xfs_last_rt_bmblock(rtg); bmbno < bmblocks; bmbno++) {
+	for (bmbno = xfs_last_rt_bmblock_to_extend(rtg); bmbno < bmblocks;
+			bmbno++) {
 		error = xfs_growfs_rt_bmblock(rtg, nrblocks, rextsize, bmbno);
 		if (error)
 			goto out_error;
-- 
2.43.5


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

* [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set
  2026-02-12 14:01 [PATCH v1 0/4] xfs: Misc changes to XFS realtime Nirjhar Roy (IBM)
  2026-02-12 14:01 ` [PATCH v1 1/4] xfs: Fix xfs_last_rt_bmblock() Nirjhar Roy (IBM)
@ 2026-02-12 14:01 ` Nirjhar Roy (IBM)
  2026-02-18  5:54   ` Christoph Hellwig
  2026-02-12 14:01 ` [PATCH v1 3/4] xfs: Update lazy counters in xfs_growfs_rt_bmblock() Nirjhar Roy (IBM)
  2026-02-12 14:01 ` [PATCH v1 4/4] xfs: Add some comments in some macros Nirjhar Roy (IBM)
  3 siblings, 1 reply; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-12 14:01 UTC (permalink / raw)
  To: djwong, hch, cem; +Cc: linux-xfs, ritesh.list, ojaswin, nirjhar.roy.lists

Since sb_frextents is a lazy counter, update it when lazy count is set,
just like sb_icount, sb_ifree and sb_fdblocks.

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
 fs/xfs/libxfs/xfs_sb.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index 94c272a2ae26..bf1f4a86dab1 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -1340,17 +1340,18 @@ xfs_log_sb(
 				percpu_counter_sum_positive(&mp->m_ifree),
 				mp->m_sb.sb_icount);
 		mp->m_sb.sb_fdblocks = xfs_sum_freecounter(mp, XC_FREE_BLOCKS);
-	}
-
-	/*
-	 * sb_frextents was added to the lazy sb counters when the rt groups
-	 * feature was introduced.  This counter can go negative due to the way
-	 * we handle nearly-lockless reservations, so we must use the _positive
-	 * variant here to avoid writing out nonsense frextents.
-	 */
-	if (xfs_has_rtgroups(mp) && !xfs_has_zoned(mp)) {
-		mp->m_sb.sb_frextents =
-				xfs_sum_freecounter(mp, XC_FREE_RTEXTENTS);
+		/*
+		 * sb_frextents was added to the lazy sb counters when the
+		 * rt groups feature was introduced.  This counter can go
+		 * negative due to the way we handle nearly-lockless
+		 * reservations, so we must use the _positive variant here to
+		 * avoid writing out nonsense frextents.
+		 */
+		if (xfs_has_rtgroups(mp) && !xfs_has_zoned(mp)) {
+			mp->m_sb.sb_frextents =
+					xfs_sum_freecounter(mp,
+					XC_FREE_RTEXTENTS);
+		}
 	}
 
 	xfs_sb_to_disk(bp->b_addr, &mp->m_sb);
-- 
2.43.5


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

* [PATCH v1 3/4] xfs: Update lazy counters in xfs_growfs_rt_bmblock()
  2026-02-12 14:01 [PATCH v1 0/4] xfs: Misc changes to XFS realtime Nirjhar Roy (IBM)
  2026-02-12 14:01 ` [PATCH v1 1/4] xfs: Fix xfs_last_rt_bmblock() Nirjhar Roy (IBM)
  2026-02-12 14:01 ` [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set Nirjhar Roy (IBM)
@ 2026-02-12 14:01 ` Nirjhar Roy (IBM)
  2026-02-18  5:55   ` Christoph Hellwig
  2026-02-12 14:01 ` [PATCH v1 4/4] xfs: Add some comments in some macros Nirjhar Roy (IBM)
  3 siblings, 1 reply; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-12 14:01 UTC (permalink / raw)
  To: djwong, hch, cem; +Cc: linux-xfs, ritesh.list, ojaswin, nirjhar.roy.lists

Update lazy counters in xfs_growfs_rt_bmblock() similar to the way it
is done xfs_growfs_data_private().

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
 fs/xfs/xfs_rtalloc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
index a7a0859ed47f..0aaa6b1afdaf 100644
--- a/fs/xfs/xfs_rtalloc.c
+++ b/fs/xfs/xfs_rtalloc.c
@@ -1047,6 +1047,9 @@ xfs_growfs_rt_bmblock(
 	 */
 	xfs_trans_resv_calc(mp, &mp->m_resv);
 
+	if (xfs_has_lazysbcount(mp))
+		xfs_log_sb(args.tp);
+
 	error = xfs_trans_commit(args.tp);
 	if (error)
 		goto out_free;
-- 
2.43.5


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

* [PATCH v1 4/4] xfs: Add some comments in some macros.
  2026-02-12 14:01 [PATCH v1 0/4] xfs: Misc changes to XFS realtime Nirjhar Roy (IBM)
                   ` (2 preceding siblings ...)
  2026-02-12 14:01 ` [PATCH v1 3/4] xfs: Update lazy counters in xfs_growfs_rt_bmblock() Nirjhar Roy (IBM)
@ 2026-02-12 14:01 ` Nirjhar Roy (IBM)
  2026-02-18  5:56   ` Christoph Hellwig
  3 siblings, 1 reply; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-12 14:01 UTC (permalink / raw)
  To: djwong, hch, cem; +Cc: linux-xfs, ritesh.list, ojaswin, nirjhar.roy.lists

Some comments on usage of XFS_IS_CORRUPT() and ASSERT.

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
---
 fs/xfs/xfs_linux.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 4dd747bdbcca..3a69dff50bfd 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -225,6 +225,7 @@ int xfs_rw_bdev(struct block_device *bdev, sector_t sector, unsigned int count,
 
 #ifdef XFS_WARN
 
+/* Please note that this ASSERT doesn't kill the kernel */
 #define ASSERT(expr)	\
 	(likely(expr) ? (void)0 : asswarn(NULL, #expr, __FILE__, __LINE__))
 
@@ -235,6 +236,11 @@ int xfs_rw_bdev(struct block_device *bdev, sector_t sector, unsigned int count,
 #endif /* XFS_WARN */
 #endif /* DEBUG */
 
+/*
+ * Use this to catch metadata corruptions that are not caught by the regular
+ * verifiers. The reason is that the verifiers check corruptions only within
+ * the block.
+ */
 #define XFS_IS_CORRUPT(mp, expr)	\
 	(unlikely(expr) ? xfs_corruption_error(#expr, XFS_ERRLEVEL_LOW, (mp), \
 					       NULL, 0, __FILE__, __LINE__, \
-- 
2.43.5


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

* Re: [PATCH v1 1/4] xfs: Fix xfs_last_rt_bmblock()
  2026-02-12 14:01 ` [PATCH v1 1/4] xfs: Fix xfs_last_rt_bmblock() Nirjhar Roy (IBM)
@ 2026-02-18  5:53   ` Christoph Hellwig
  2026-02-18  7:47     ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2026-02-18  5:53 UTC (permalink / raw)
  To: Nirjhar Roy (IBM); +Cc: djwong, hch, cem, linux-xfs, ritesh.list, ojaswin

On Thu, Feb 12, 2026 at 07:31:44PM +0530, Nirjhar Roy (IBM) wrote:
> Reproduce:
> $ mkfs.xfs -m metadir=0 -r rtdev=/dev/loop1 /dev/loop0 \
> 	-r rgsize=32768b,size=32769b -f
> $ mount -o rtdev=/dev/loop1 /dev/loop0 /mnt/scratch
> $ xfs_growfs -R $(( 32769 + 1 )) /mnt/scratch
> $ xfs_info /mnt/scratch | grep rtextents
> $ # We can see that rtextents hasn't changed

Please wire this up in xfstests.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set
  2026-02-12 14:01 ` [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set Nirjhar Roy (IBM)
@ 2026-02-18  5:54   ` Christoph Hellwig
  2026-02-18  8:49     ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2026-02-18  5:54 UTC (permalink / raw)
  To: Nirjhar Roy (IBM); +Cc: djwong, hch, cem, linux-xfs, ritesh.list, ojaswin

On Thu, Feb 12, 2026 at 07:31:45PM +0530, Nirjhar Roy (IBM) wrote:
> Since sb_frextents is a lazy counter, update it when lazy count is set,
> just like sb_icount, sb_ifree and sb_fdblocks.

The comment you removed explains why we need a different conditional
for it, though.

The commit message also doesn't explain at all:

 - why you want to change it
 - what the chane is (AFAICS just the conditional and not how it is
   updated)

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

* Re: [PATCH v1 3/4] xfs: Update lazy counters in xfs_growfs_rt_bmblock()
  2026-02-12 14:01 ` [PATCH v1 3/4] xfs: Update lazy counters in xfs_growfs_rt_bmblock() Nirjhar Roy (IBM)
@ 2026-02-18  5:55   ` Christoph Hellwig
  2026-02-18  7:48     ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2026-02-18  5:55 UTC (permalink / raw)
  To: Nirjhar Roy (IBM); +Cc: djwong, hch, cem, linux-xfs, ritesh.list, ojaswin

On Thu, Feb 12, 2026 at 07:31:46PM +0530, Nirjhar Roy (IBM) wrote:
> Update lazy counters in xfs_growfs_rt_bmblock() similar to the way it
> is done xfs_growfs_data_private().

Please explain why you're doing this.  I think it would also be helpful
to bring over a comment similar to the one in xfs_growfs_data_private.


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

* Re: [PATCH v1 4/4] xfs: Add some comments in some macros.
  2026-02-12 14:01 ` [PATCH v1 4/4] xfs: Add some comments in some macros Nirjhar Roy (IBM)
@ 2026-02-18  5:56   ` Christoph Hellwig
  2026-02-18  7:50     ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2026-02-18  5:56 UTC (permalink / raw)
  To: Nirjhar Roy (IBM); +Cc: djwong, hch, cem, linux-xfs, ritesh.list, ojaswin

On Thu, Feb 12, 2026 at 07:31:47PM +0530, Nirjhar Roy (IBM) wrote:
> Some comments on usage of XFS_IS_CORRUPT() and ASSERT.

"Add comments explaining when to use XFS_IS_CORRUPT() and ASSERT()"

?

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>


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

* Re: [PATCH v1 1/4] xfs: Fix xfs_last_rt_bmblock()
  2026-02-18  5:53   ` Christoph Hellwig
@ 2026-02-18  7:47     ` Nirjhar Roy (IBM)
  0 siblings, 0 replies; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-18  7:47 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: djwong, cem, linux-xfs, ritesh.list, ojaswin


On 2/18/26 11:23, Christoph Hellwig wrote:
> On Thu, Feb 12, 2026 at 07:31:44PM +0530, Nirjhar Roy (IBM) wrote:
>> Reproduce:
>> $ mkfs.xfs -m metadir=0 -r rtdev=/dev/loop1 /dev/loop0 \
>> 	-r rgsize=32768b,size=32769b -f
>> $ mount -o rtdev=/dev/loop1 /dev/loop0 /mnt/scratch
>> $ xfs_growfs -R $(( 32769 + 1 )) /mnt/scratch
>> $ xfs_info /mnt/scratch | grep rtextents
>> $ # We can see that rtextents hasn't changed
> Please wire this up in xfstests.

Yes, I have prepared a bunch of XFS realtime grow and shrink tests as a 
part of XFS realtime shrinkfs work. That will cover this fix (and a 
couple of other fixes that I posted earlier). I will send them shortly 
in a few days.

>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thank you.

--NR

-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


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

* Re: [PATCH v1 3/4] xfs: Update lazy counters in xfs_growfs_rt_bmblock()
  2026-02-18  5:55   ` Christoph Hellwig
@ 2026-02-18  7:48     ` Nirjhar Roy (IBM)
  0 siblings, 0 replies; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-18  7:48 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: djwong, cem, linux-xfs, ritesh.list, ojaswin


On 2/18/26 11:25, Christoph Hellwig wrote:
> On Thu, Feb 12, 2026 at 07:31:46PM +0530, Nirjhar Roy (IBM) wrote:
>> Update lazy counters in xfs_growfs_rt_bmblock() similar to the way it
>> is done xfs_growfs_data_private().
> Please explain why you're doing this.  I think it would also be helpful
> to bring over a comment similar to the one in xfs_growfs_data_private.

Sure. Thank you for the suggestion.

--NR

>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


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

* Re: [PATCH v1 4/4] xfs: Add some comments in some macros.
  2026-02-18  5:56   ` Christoph Hellwig
@ 2026-02-18  7:50     ` Nirjhar Roy (IBM)
  0 siblings, 0 replies; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-18  7:50 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: djwong, cem, linux-xfs, ritesh.list, ojaswin


On 2/18/26 11:26, Christoph Hellwig wrote:
> On Thu, Feb 12, 2026 at 07:31:47PM +0530, Nirjhar Roy (IBM) wrote:
>> Some comments on usage of XFS_IS_CORRUPT() and ASSERT.
> "Add comments explaining when to use XFS_IS_CORRUPT() and ASSERT()"
>
> ?

Sure, I will make the change.

--NR

>
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


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

* Re: [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set
  2026-02-18  5:54   ` Christoph Hellwig
@ 2026-02-18  8:49     ` Nirjhar Roy (IBM)
  2026-02-19  6:38       ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-18  8:49 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: djwong, cem, linux-xfs, ritesh.list, ojaswin

On Tue, 2026-02-17 at 21:54 -0800, Christoph Hellwig wrote:
> On Thu, Feb 12, 2026 at 07:31:45PM +0530, Nirjhar Roy (IBM) wrote:
> > Since sb_frextents is a lazy counter, update it when lazy count is set,
> > just like sb_icount, sb_ifree and sb_fdblocks.
> 
> The comment you removed explains why we need a different conditional
> for it, though.
So I just moved the comment and the updation of sb_frextents from outside of "if
(xfs_has_lazysbcount(mp)) {" to inside of it since sb_frextents is a also a lazy counter like the
sb_fdblocks, sb_ifree. The comment talks about using the positive version of freecounter i.e,
xfs_sum_freecounter(). Do you mean to say that the updation/sync of the sb_frextents should be
outside "if (xfs_has_lazysbcount(mp)) {" i.e, done irrespective of whether lazy count is set or not?
Please let me know if my understanding is wrong.
> 
> The commit message also doesn't explain at all:
Okay I can update the commit message
> 
>  - why you want to change it
How about "We should update all the free counters only if lazy count is set, else it will be
unnecessary work"?
>  - what the chane is (AFAICS just the conditional and not how it is
>    updated)
How about "Updating sb_frextents conditionally based on whether lazy count it set instead of doing
it unconditionally everytime when xfs_log_sb() is called"?
Does the above 2 look fine?
--NR


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

* Re: [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set
  2026-02-18  8:49     ` Nirjhar Roy (IBM)
@ 2026-02-19  6:38       ` Christoph Hellwig
  2026-02-19  6:46         ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2026-02-19  6:38 UTC (permalink / raw)
  To: Nirjhar Roy (IBM)
  Cc: Christoph Hellwig, djwong, cem, linux-xfs, ritesh.list, ojaswin

On Wed, Feb 18, 2026 at 02:19:21PM +0530, Nirjhar Roy (IBM) wrote:
> So I just moved the comment and the updation of sb_frextents from
> outside of "if (xfs_has_lazysbcount(mp)) {" to inside of it since
> sb_frextents is a also a lazy counter like the sb_fdblocks, sb_ifree.
> The comment talks about using the positive version of freecounter i.e,
> xfs_sum_freecounter(). Do you mean to say that the updation/sync of the sb_frextents should be
> outside "if (xfs_has_lazysbcount(mp)) {" i.e, done irrespective of whether lazy count is set or not?
> Please let me know if my understanding is wrong.

rtgroup support requires lazy counters.  So we don't really need that
clause, and the extra indentation makes the code harder to read (
to the point that I mised that you actually kept the correct check).


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

* Re: [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set
  2026-02-19  6:38       ` Christoph Hellwig
@ 2026-02-19  6:46         ` Nirjhar Roy (IBM)
  2026-02-19  6:51           ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-19  6:46 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: djwong, cem, linux-xfs, ritesh.list, ojaswin


On 2/19/26 12:08, Christoph Hellwig wrote:
> On Wed, Feb 18, 2026 at 02:19:21PM +0530, Nirjhar Roy (IBM) wrote:
>> So I just moved the comment and the updation of sb_frextents from
>> outside of "if (xfs_has_lazysbcount(mp)) {" to inside of it since
>> sb_frextents is a also a lazy counter like the sb_fdblocks, sb_ifree.
>> The comment talks about using the positive version of freecounter i.e,
>> xfs_sum_freecounter(). Do you mean to say that the updation/sync of the sb_frextents should be
>> outside "if (xfs_has_lazysbcount(mp)) {" i.e, done irrespective of whether lazy count is set or not?
>> Please let me know if my understanding is wrong.
> rtgroup support requires lazy counters.  So we don't really need that
> clause, and the extra indentation makes the code harder to read (
> to the point that I mised that you actually kept the correct check).

Okay got it. So what you are saying is that, for rtgroup support, lazy 
counter is ALWAYS enabled and xfs_has_lazysbcount(mp) will always be 
true - so there is no need to keep the updation of sb_frextents inside 
the if() block. Right?

--NR

>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


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

* Re: [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set
  2026-02-19  6:46         ` Nirjhar Roy (IBM)
@ 2026-02-19  6:51           ` Christoph Hellwig
  2026-02-19  7:05             ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2026-02-19  6:51 UTC (permalink / raw)
  To: Nirjhar Roy (IBM)
  Cc: Christoph Hellwig, djwong, cem, linux-xfs, ritesh.list, ojaswin

On Thu, Feb 19, 2026 at 12:16:51PM +0530, Nirjhar Roy (IBM) wrote:
> Okay got it. So what you are saying is that, for rtgroup support, lazy
> counter is ALWAYS enabled and xfs_has_lazysbcount(mp) will always be true -
> so there is no need to keep the updation of sb_frextents inside the if()
> block. Right?

Yes.  And given that it confused you, maybe add a comment about that
for the next round:

	/*
	 * RT groups are only supported on v5 file systems, which always
	 * have lazy SB counters.
	 */


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

* Re: [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set
  2026-02-19  6:51           ` Christoph Hellwig
@ 2026-02-19  7:05             ` Nirjhar Roy (IBM)
  2026-02-19  7:07               ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-19  7:05 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: djwong, cem, linux-xfs, ritesh.list, ojaswin


On 2/19/26 12:21, Christoph Hellwig wrote:
> On Thu, Feb 19, 2026 at 12:16:51PM +0530, Nirjhar Roy (IBM) wrote:
>> Okay got it. So what you are saying is that, for rtgroup support, lazy
>> counter is ALWAYS enabled and xfs_has_lazysbcount(mp) will always be true -
>> so there is no need to keep the updation of sb_frextents inside the if()
>> block. Right?
> Yes.  And given that it confused you, maybe add a comment about that
> for the next round:
>
> 	/*
> 	 * RT groups are only supported on v5 file systems, which always
> 	 * have lazy SB counters.
> 	 */

Okay, got it. Thank you for the explanation. So I guess the reason for 
always keeping lazy counters enabled is for performance? So that on 
every update of the counters, we don't go to the disk and increase latency?

--NR

>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


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

* Re: [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set
  2026-02-19  7:05             ` Nirjhar Roy (IBM)
@ 2026-02-19  7:07               ` Christoph Hellwig
  2026-02-19  7:09                 ` Nirjhar Roy (IBM)
  0 siblings, 1 reply; 19+ messages in thread
From: Christoph Hellwig @ 2026-02-19  7:07 UTC (permalink / raw)
  To: Nirjhar Roy (IBM)
  Cc: Christoph Hellwig, djwong, cem, linux-xfs, ritesh.list, ojaswin

On Thu, Feb 19, 2026 at 12:35:49PM +0530, Nirjhar Roy (IBM) wrote:
> Okay, got it. Thank you for the explanation. So I guess the reason for
> always keeping lazy counters enabled is for performance? So that on every
> update of the counters, we don't go to the disk and increase latency?

Yes.  They are generally a much better scheme, but it took the XFS
developers a while to figure that out :)


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

* Re: [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set
  2026-02-19  7:07               ` Christoph Hellwig
@ 2026-02-19  7:09                 ` Nirjhar Roy (IBM)
  0 siblings, 0 replies; 19+ messages in thread
From: Nirjhar Roy (IBM) @ 2026-02-19  7:09 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: djwong, cem, linux-xfs, ritesh.list, ojaswin


On 2/19/26 12:37, Christoph Hellwig wrote:
> On Thu, Feb 19, 2026 at 12:35:49PM +0530, Nirjhar Roy (IBM) wrote:
>> Okay, got it. Thank you for the explanation. So I guess the reason for
>> always keeping lazy counters enabled is for performance? So that on every
>> update of the counters, we don't go to the disk and increase latency?
> Yes.  They are generally a much better scheme, but it took the XFS
> developers a while to figure that out :)

Okay. Sure, I will update these in the next revision.

--NR

>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


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

end of thread, other threads:[~2026-02-19  7:09 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-12 14:01 [PATCH v1 0/4] xfs: Misc changes to XFS realtime Nirjhar Roy (IBM)
2026-02-12 14:01 ` [PATCH v1 1/4] xfs: Fix xfs_last_rt_bmblock() Nirjhar Roy (IBM)
2026-02-18  5:53   ` Christoph Hellwig
2026-02-18  7:47     ` Nirjhar Roy (IBM)
2026-02-12 14:01 ` [PATCH v1 2/4] xfs: Update sb_frextents when lazy count is set Nirjhar Roy (IBM)
2026-02-18  5:54   ` Christoph Hellwig
2026-02-18  8:49     ` Nirjhar Roy (IBM)
2026-02-19  6:38       ` Christoph Hellwig
2026-02-19  6:46         ` Nirjhar Roy (IBM)
2026-02-19  6:51           ` Christoph Hellwig
2026-02-19  7:05             ` Nirjhar Roy (IBM)
2026-02-19  7:07               ` Christoph Hellwig
2026-02-19  7:09                 ` Nirjhar Roy (IBM)
2026-02-12 14:01 ` [PATCH v1 3/4] xfs: Update lazy counters in xfs_growfs_rt_bmblock() Nirjhar Roy (IBM)
2026-02-18  5:55   ` Christoph Hellwig
2026-02-18  7:48     ` Nirjhar Roy (IBM)
2026-02-12 14:01 ` [PATCH v1 4/4] xfs: Add some comments in some macros Nirjhar Roy (IBM)
2026-02-18  5:56   ` Christoph Hellwig
2026-02-18  7:50     ` Nirjhar Roy (IBM)

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