[PATCH v2] ext4: fix dirtyclusters double decrement on fs shutdown

[PATCH v2] ext4: fix dirtyclusters double decrement on fs shutdown

[Brian Foster]

fstests test generic/388 occasionally reproduces a warning in
ext4_put_super() associated with the dirty clusters count:

  WARNING: CPU: 7 PID: 76064 at fs/ext4/super.c:1324 ext4_put_super+0x48c/0x590 [ext4]

Tracing the failure shows that the warning fires due to an
s_dirtyclusters_counter value of -1. IOW, this appears to be a
spurious decrement as opposed to some sort of leak. Further tracing
of the dirty cluster count deltas and an LLM scan of the resulting
output identified the cause as a double decrement in the error path
between ext4_mb_mark_diskspace_used() and the caller
ext4_mb_new_blocks().

First, note that generic/388 is a shutdown vs. fsstress test and so
produces a random set of operations and shutdown injections. In the
problematic case, the shutdown triggers an error return from the
ext4_handle_dirty_metadata() call(s) made from
ext4_mb_mark_context(). The changed value is non-zero at this point,
so ext4_mb_mark_diskspace_used() does not exit after the error
bubbles up from ext4_mb_mark_context(). Instead, the former
decrements both cluster counters and returns the error up to
ext4_mb_new_blocks(). The latter falls into the !ar->len out path
which decrements the dirty clusters counter a second time, creating
the inconsistency.

To avoid this problem and simplify ownership of the cluster
reservation in this codepath, lift the counter reduction to a single
place in the caller. This makes it more clear that
ext4_mb_new_blocks() is responsible for acquiring cluster
reservation (via ext4_claim_free_clusters()) in the !delalloc case
as well as releasing it, regardless of whether it ends up consumed
or returned due to failure.

Fixes: 0087d9fb3f29 ("ext4: Fix s_dirty_blocks_counter if block allocation failed with nodelalloc")
Signed-off-by: Brian Foster <bfoster@redhat.com>
---

v2:
- Condense counter update logic instead of modifying return flow.
- Added Fixes: tag.
v1: https://lore.kernel.org/linux-ext4/20251212154735.512651-1-bfoster@redhat.com/

 fs/ext4/mballoc.c | 21 +++++----------------
 1 file changed, 5 insertions(+), 16 deletions(-)

diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 56d50fd3310b..b31d7ddc52a9 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4185,8 +4185,7 @@ ext4_mb_mark_context(handle_t *handle, struct super_block *sb, bool state,
  * Returns 0 if success or error code
  */
 static noinline_for_stack int
-ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
-				handle_t *handle, unsigned int reserv_clstrs)
+ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, handle_t *handle)
 {
 	struct ext4_group_desc *gdp;
 	struct ext4_sb_info *sbi;
@@ -4241,13 +4240,6 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
 	BUG_ON(changed != ac->ac_b_ex.fe_len);
 #endif
 	percpu_counter_sub(&sbi->s_freeclusters_counter, ac->ac_b_ex.fe_len);
-	/*
-	 * Now reduce the dirty block count also. Should not go negative
-	 */
-	if (!(ac->ac_flags & EXT4_MB_DELALLOC_RESERVED))
-		/* release all the reserved blocks if non delalloc */
-		percpu_counter_sub(&sbi->s_dirtyclusters_counter,
-				   reserv_clstrs);
 
 	return err;
 }
@@ -6332,7 +6324,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
 			ext4_mb_pa_put_free(ac);
 	}
 	if (likely(ac->ac_status == AC_STATUS_FOUND)) {
-		*errp = ext4_mb_mark_diskspace_used(ac, handle, reserv_clstrs);
+		*errp = ext4_mb_mark_diskspace_used(ac, handle);
 		if (*errp) {
 			ext4_discard_allocated_blocks(ac);
 			goto errout;
@@ -6363,12 +6355,9 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
 out:
 	if (inquota && ar->len < inquota)
 		dquot_free_block(ar->inode, EXT4_C2B(sbi, inquota - ar->len));
-	if (!ar->len) {
-		if ((ar->flags & EXT4_MB_DELALLOC_RESERVED) == 0)
-			/* release all the reserved blocks if non delalloc */
-			percpu_counter_sub(&sbi->s_dirtyclusters_counter,
-						reserv_clstrs);
-	}
+	/* release all the reserved blocks if non delalloc */
+	if ((ar->flags & EXT4_MB_DELALLOC_RESERVED) == 0)
+		percpu_counter_sub(&sbi->s_dirtyclusters_counter, reserv_clstrs);
 
 	trace_ext4_allocate_blocks(ar, (unsigned long long)block);
 
-- 
2.52.0

Re: [PATCH v2] ext4: fix dirtyclusters double decrement on fs shutdown

[kernel test robot]

Hi Brian,

kernel test robot noticed the following build errors:

[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on linus/master v6.19-rc5 next-20260109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Brian-Foster/ext4-fix-dirtyclusters-double-decrement-on-fs-shutdown/20260112-225259
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20260112143652.8085-1-bfoster%40redhat.com
patch subject: [PATCH v2] ext4: fix dirtyclusters double decrement on fs shutdown
config: arm64-randconfig-002-20260113 (https://download.01.org/0day-ci/archive/20260113/202601130527.WKPDXmMv-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 9b8addffa70cee5b2acc5454712d9cf78ce45710)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260113/202601130527.WKPDXmMv-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601130527.WKPDXmMv-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from fs/ext4/mballoc.c:7185:
>> fs/ext4/mballoc-test.c:570:46: error: too many arguments to function call, expected 2, have 3
     570 |         ret = ext4_mb_mark_diskspace_used(ac, NULL, 0);
         |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~           ^
   fs/ext4/mballoc.c:4188:1: note: 'ext4_mb_mark_diskspace_used' declared here
    4188 | ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, handle_t *handle)
         | ^                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +570 fs/ext4/mballoc-test.c

6c5e0c9c21456f Kemeng Shi 2024-01-03  548  
2b81493f8eb6fc Kemeng Shi 2024-01-03  549  static void
2b81493f8eb6fc Kemeng Shi 2024-01-03  550  test_mark_diskspace_used_range(struct kunit *test,
2b81493f8eb6fc Kemeng Shi 2024-01-03  551  			       struct ext4_allocation_context *ac,
2b81493f8eb6fc Kemeng Shi 2024-01-03  552  			       ext4_grpblk_t start,
2b81493f8eb6fc Kemeng Shi 2024-01-03  553  			       ext4_grpblk_t len)
2b81493f8eb6fc Kemeng Shi 2024-01-03  554  {
2b81493f8eb6fc Kemeng Shi 2024-01-03  555  	struct super_block *sb = (struct super_block *)test->priv;
2b81493f8eb6fc Kemeng Shi 2024-01-03  556  	int ret;
2b81493f8eb6fc Kemeng Shi 2024-01-03  557  	void *bitmap;
2b81493f8eb6fc Kemeng Shi 2024-01-03  558  	ext4_grpblk_t i, max;
2b81493f8eb6fc Kemeng Shi 2024-01-03  559  
2b81493f8eb6fc Kemeng Shi 2024-01-03  560  	/* ext4_mb_mark_diskspace_used will BUG if len is 0 */
2b81493f8eb6fc Kemeng Shi 2024-01-03  561  	if (len == 0)
2b81493f8eb6fc Kemeng Shi 2024-01-03  562  		return;
2b81493f8eb6fc Kemeng Shi 2024-01-03  563  
2b81493f8eb6fc Kemeng Shi 2024-01-03  564  	ac->ac_b_ex.fe_group = TEST_GOAL_GROUP;
2b81493f8eb6fc Kemeng Shi 2024-01-03  565  	ac->ac_b_ex.fe_start = start;
2b81493f8eb6fc Kemeng Shi 2024-01-03  566  	ac->ac_b_ex.fe_len = len;
2b81493f8eb6fc Kemeng Shi 2024-01-03  567  
2b81493f8eb6fc Kemeng Shi 2024-01-03  568  	bitmap = mbt_ctx_bitmap(sb, TEST_GOAL_GROUP);
2b81493f8eb6fc Kemeng Shi 2024-01-03  569  	memset(bitmap, 0, sb->s_blocksize);
2b81493f8eb6fc Kemeng Shi 2024-01-03 @570  	ret = ext4_mb_mark_diskspace_used(ac, NULL, 0);
2b81493f8eb6fc Kemeng Shi 2024-01-03  571  	KUNIT_ASSERT_EQ(test, ret, 0);
2b81493f8eb6fc Kemeng Shi 2024-01-03  572  
2b81493f8eb6fc Kemeng Shi 2024-01-03  573  	max = EXT4_CLUSTERS_PER_GROUP(sb);
2b81493f8eb6fc Kemeng Shi 2024-01-03  574  	i = mb_find_next_bit(bitmap, max, 0);
2b81493f8eb6fc Kemeng Shi 2024-01-03  575  	KUNIT_ASSERT_EQ(test, i, start);
2b81493f8eb6fc Kemeng Shi 2024-01-03  576  	i = mb_find_next_zero_bit(bitmap, max, i + 1);
2b81493f8eb6fc Kemeng Shi 2024-01-03  577  	KUNIT_ASSERT_EQ(test, i, start + len);
2b81493f8eb6fc Kemeng Shi 2024-01-03  578  	i = mb_find_next_bit(bitmap, max, i + 1);
2b81493f8eb6fc Kemeng Shi 2024-01-03  579  	KUNIT_ASSERT_EQ(test, max, i);
2b81493f8eb6fc Kemeng Shi 2024-01-03  580  }
2b81493f8eb6fc Kemeng Shi 2024-01-03  581  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] ext4: fix dirtyclusters double decrement on fs shutdown

[kernel test robot]

Hi Brian,

kernel test robot noticed the following build errors:

[auto build test ERROR on tytso-ext4/dev]
[also build test ERROR on linus/master v6.19-rc5 next-20260109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Brian-Foster/ext4-fix-dirtyclusters-double-decrement-on-fs-shutdown/20260112-225259
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
patch link:    https://lore.kernel.org/r/20260112143652.8085-1-bfoster%40redhat.com
patch subject: [PATCH v2] ext4: fix dirtyclusters double decrement on fs shutdown
config: arm64-randconfig-004-20260113 (https://download.01.org/0day-ci/archive/20260113/202601130514.GnuB5QCD-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 12.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260113/202601130514.GnuB5QCD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202601130514.GnuB5QCD-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from fs/ext4/mballoc.c:7185:
   fs/ext4/mballoc-test.c: In function 'test_mark_diskspace_used_range':
>> fs/ext4/mballoc-test.c:570:15: error: too many arguments to function 'ext4_mb_mark_diskspace_used'
     570 |         ret = ext4_mb_mark_diskspace_used(ac, NULL, 0);
         |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/ext4/mballoc.c:4188:1: note: declared here
    4188 | ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, handle_t *handle)
         | ^~~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/ext4_mb_mark_diskspace_used +570 fs/ext4/mballoc-test.c

6c5e0c9c21456f Kemeng Shi 2024-01-03  548  
2b81493f8eb6fc Kemeng Shi 2024-01-03  549  static void
2b81493f8eb6fc Kemeng Shi 2024-01-03  550  test_mark_diskspace_used_range(struct kunit *test,
2b81493f8eb6fc Kemeng Shi 2024-01-03  551  			       struct ext4_allocation_context *ac,
2b81493f8eb6fc Kemeng Shi 2024-01-03  552  			       ext4_grpblk_t start,
2b81493f8eb6fc Kemeng Shi 2024-01-03  553  			       ext4_grpblk_t len)
2b81493f8eb6fc Kemeng Shi 2024-01-03  554  {
2b81493f8eb6fc Kemeng Shi 2024-01-03  555  	struct super_block *sb = (struct super_block *)test->priv;
2b81493f8eb6fc Kemeng Shi 2024-01-03  556  	int ret;
2b81493f8eb6fc Kemeng Shi 2024-01-03  557  	void *bitmap;
2b81493f8eb6fc Kemeng Shi 2024-01-03  558  	ext4_grpblk_t i, max;
2b81493f8eb6fc Kemeng Shi 2024-01-03  559  
2b81493f8eb6fc Kemeng Shi 2024-01-03  560  	/* ext4_mb_mark_diskspace_used will BUG if len is 0 */
2b81493f8eb6fc Kemeng Shi 2024-01-03  561  	if (len == 0)
2b81493f8eb6fc Kemeng Shi 2024-01-03  562  		return;
2b81493f8eb6fc Kemeng Shi 2024-01-03  563  
2b81493f8eb6fc Kemeng Shi 2024-01-03  564  	ac->ac_b_ex.fe_group = TEST_GOAL_GROUP;
2b81493f8eb6fc Kemeng Shi 2024-01-03  565  	ac->ac_b_ex.fe_start = start;
2b81493f8eb6fc Kemeng Shi 2024-01-03  566  	ac->ac_b_ex.fe_len = len;
2b81493f8eb6fc Kemeng Shi 2024-01-03  567  
2b81493f8eb6fc Kemeng Shi 2024-01-03  568  	bitmap = mbt_ctx_bitmap(sb, TEST_GOAL_GROUP);
2b81493f8eb6fc Kemeng Shi 2024-01-03  569  	memset(bitmap, 0, sb->s_blocksize);
2b81493f8eb6fc Kemeng Shi 2024-01-03 @570  	ret = ext4_mb_mark_diskspace_used(ac, NULL, 0);
2b81493f8eb6fc Kemeng Shi 2024-01-03  571  	KUNIT_ASSERT_EQ(test, ret, 0);
2b81493f8eb6fc Kemeng Shi 2024-01-03  572  
2b81493f8eb6fc Kemeng Shi 2024-01-03  573  	max = EXT4_CLUSTERS_PER_GROUP(sb);
2b81493f8eb6fc Kemeng Shi 2024-01-03  574  	i = mb_find_next_bit(bitmap, max, 0);
2b81493f8eb6fc Kemeng Shi 2024-01-03  575  	KUNIT_ASSERT_EQ(test, i, start);
2b81493f8eb6fc Kemeng Shi 2024-01-03  576  	i = mb_find_next_zero_bit(bitmap, max, i + 1);
2b81493f8eb6fc Kemeng Shi 2024-01-03  577  	KUNIT_ASSERT_EQ(test, i, start + len);
2b81493f8eb6fc Kemeng Shi 2024-01-03  578  	i = mb_find_next_bit(bitmap, max, i + 1);
2b81493f8eb6fc Kemeng Shi 2024-01-03  579  	KUNIT_ASSERT_EQ(test, max, i);
2b81493f8eb6fc Kemeng Shi 2024-01-03  580  }
2b81493f8eb6fc Kemeng Shi 2024-01-03  581  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2] ext4: fix dirtyclusters double decrement on fs shutdown

[Baokun Li]

On 2026-01-12 22:36, Brian Foster wrote:
> fstests test generic/388 occasionally reproduces a warning in
> ext4_put_super() associated with the dirty clusters count:
>
>   WARNING: CPU: 7 PID: 76064 at fs/ext4/super.c:1324 ext4_put_super+0x48c/0x590 [ext4]
>
> Tracing the failure shows that the warning fires due to an
> s_dirtyclusters_counter value of -1. IOW, this appears to be a
> spurious decrement as opposed to some sort of leak. Further tracing
> of the dirty cluster count deltas and an LLM scan of the resulting
> output identified the cause as a double decrement in the error path
> between ext4_mb_mark_diskspace_used() and the caller
> ext4_mb_new_blocks().
>
> First, note that generic/388 is a shutdown vs. fsstress test and so
> produces a random set of operations and shutdown injections. In the
> problematic case, the shutdown triggers an error return from the
> ext4_handle_dirty_metadata() call(s) made from
> ext4_mb_mark_context(). The changed value is non-zero at this point,
> so ext4_mb_mark_diskspace_used() does not exit after the error
> bubbles up from ext4_mb_mark_context(). Instead, the former
> decrements both cluster counters and returns the error up to
> ext4_mb_new_blocks(). The latter falls into the !ar->len out path
> which decrements the dirty clusters counter a second time, creating
> the inconsistency.
>
> To avoid this problem and simplify ownership of the cluster
> reservation in this codepath, lift the counter reduction to a single
> place in the caller. This makes it more clear that
> ext4_mb_new_blocks() is responsible for acquiring cluster
> reservation (via ext4_claim_free_clusters()) in the !delalloc case
> as well as releasing it, regardless of whether it ends up consumed
> or returned due to failure.
>
> Fixes: 0087d9fb3f29 ("ext4: Fix s_dirty_blocks_counter if block allocation failed with nodelalloc")
> Signed-off-by: Brian Foster <bfoster@redhat.com>

Thanks for the patch.

However, the call site in test_mark_diskspace_used_range() missed the
argument update, which triggered a Kernel Test Robot warning. Also,
I have one nit below.

> ---
>
> v2:
> - Condense counter update logic instead of modifying return flow.
> - Added Fixes: tag.
> v1: https://lore.kernel.org/linux-ext4/20251212154735.512651-1-bfoster@redhat.com/
>
>  fs/ext4/mballoc.c | 21 +++++----------------
>  1 file changed, 5 insertions(+), 16 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 56d50fd3310b..b31d7ddc52a9 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4185,8 +4185,7 @@ ext4_mb_mark_context(handle_t *handle, struct super_block *sb, bool state,
>   * Returns 0 if success or error code
>   */
>  static noinline_for_stack int
> -ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
> -				handle_t *handle, unsigned int reserv_clstrs)
> +ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, handle_t *handle)
>  {
>  	struct ext4_group_desc *gdp;
>  	struct ext4_sb_info *sbi;
> @@ -4241,13 +4240,6 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
>  	BUG_ON(changed != ac->ac_b_ex.fe_len);
>  #endif
>  	percpu_counter_sub(&sbi->s_freeclusters_counter, ac->ac_b_ex.fe_len);
> -	/*
> -	 * Now reduce the dirty block count also. Should not go negative
> -	 */
> -	if (!(ac->ac_flags & EXT4_MB_DELALLOC_RESERVED))
> -		/* release all the reserved blocks if non delalloc */
> -		percpu_counter_sub(&sbi->s_dirtyclusters_counter,
> -				   reserv_clstrs);
>  
>  	return err;
>  }
> @@ -6332,7 +6324,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
>  			ext4_mb_pa_put_free(ac);
>  	}
>  	if (likely(ac->ac_status == AC_STATUS_FOUND)) {
> -		*errp = ext4_mb_mark_diskspace_used(ac, handle, reserv_clstrs);
> +		*errp = ext4_mb_mark_diskspace_used(ac, handle);
>  		if (*errp) {
>  			ext4_discard_allocated_blocks(ac);
>  			goto errout;
> @@ -6363,12 +6355,9 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
>  out:
>  	if (inquota && ar->len < inquota)
>  		dquot_free_block(ar->inode, EXT4_C2B(sbi, inquota - ar->len));
> -	if (!ar->len) {
> -		if ((ar->flags & EXT4_MB_DELALLOC_RESERVED) == 0)
> -			/* release all the reserved blocks if non delalloc */
> -			percpu_counter_sub(&sbi->s_dirtyclusters_counter,
> -						reserv_clstrs);
> -	}
> +	/* release all the reserved blocks if non delalloc */
> +	if ((ar->flags & EXT4_MB_DELALLOC_RESERVED) == 0)

Nit: It might be better to check if (reserv_clstrs) directly. It’s more
straightforward and stays robust even if the flag logic changes later.

> +		percpu_counter_sub(&sbi->s_dirtyclusters_counter, reserv_clstrs);
>  
>  	trace_ext4_allocate_blocks(ar, (unsigned long long)block);
>  

Re: [PATCH v2] ext4: fix dirtyclusters double decrement on fs shutdown

[Brian Foster]

On Tue, Jan 13, 2026 at 09:44:16AM +0800, Baokun Li wrote:
> On 2026-01-12 22:36, Brian Foster wrote:
> > fstests test generic/388 occasionally reproduces a warning in
> > ext4_put_super() associated with the dirty clusters count:
> >
> >   WARNING: CPU: 7 PID: 76064 at fs/ext4/super.c:1324 ext4_put_super+0x48c/0x590 [ext4]
> >
> > Tracing the failure shows that the warning fires due to an
> > s_dirtyclusters_counter value of -1. IOW, this appears to be a
> > spurious decrement as opposed to some sort of leak. Further tracing
> > of the dirty cluster count deltas and an LLM scan of the resulting
> > output identified the cause as a double decrement in the error path
> > between ext4_mb_mark_diskspace_used() and the caller
> > ext4_mb_new_blocks().
> >
> > First, note that generic/388 is a shutdown vs. fsstress test and so
> > produces a random set of operations and shutdown injections. In the
> > problematic case, the shutdown triggers an error return from the
> > ext4_handle_dirty_metadata() call(s) made from
> > ext4_mb_mark_context(). The changed value is non-zero at this point,
> > so ext4_mb_mark_diskspace_used() does not exit after the error
> > bubbles up from ext4_mb_mark_context(). Instead, the former
> > decrements both cluster counters and returns the error up to
> > ext4_mb_new_blocks(). The latter falls into the !ar->len out path
> > which decrements the dirty clusters counter a second time, creating
> > the inconsistency.
> >
> > To avoid this problem and simplify ownership of the cluster
> > reservation in this codepath, lift the counter reduction to a single
> > place in the caller. This makes it more clear that
> > ext4_mb_new_blocks() is responsible for acquiring cluster
> > reservation (via ext4_claim_free_clusters()) in the !delalloc case
> > as well as releasing it, regardless of whether it ends up consumed
> > or returned due to failure.
> >
> > Fixes: 0087d9fb3f29 ("ext4: Fix s_dirty_blocks_counter if block allocation failed with nodelalloc")
> > Signed-off-by: Brian Foster <bfoster@redhat.com>
> 
> Thanks for the patch.
> 
> However, the call site in test_mark_diskspace_used_range() missed the
> argument update, which triggered a Kernel Test Robot warning. Also,
> I have one nit below.

Ugh yeah, I guess I didn't have KUNIT enabled so I missed that one. Will
fix.

> 
> > ---
> >
> > v2:
> > - Condense counter update logic instead of modifying return flow.
> > - Added Fixes: tag.
> > v1: https://lore.kernel.org/linux-ext4/20251212154735.512651-1-bfoster@redhat.com/
> >
> >  fs/ext4/mballoc.c | 21 +++++----------------
> >  1 file changed, 5 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> > index 56d50fd3310b..b31d7ddc52a9 100644
> > --- a/fs/ext4/mballoc.c
> > +++ b/fs/ext4/mballoc.c
> > @@ -4185,8 +4185,7 @@ ext4_mb_mark_context(handle_t *handle, struct super_block *sb, bool state,
> >   * Returns 0 if success or error code
> >   */
> >  static noinline_for_stack int
> > -ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
> > -				handle_t *handle, unsigned int reserv_clstrs)
> > +ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, handle_t *handle)
> >  {
> >  	struct ext4_group_desc *gdp;
> >  	struct ext4_sb_info *sbi;
> > @@ -4241,13 +4240,6 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
> >  	BUG_ON(changed != ac->ac_b_ex.fe_len);
> >  #endif
> >  	percpu_counter_sub(&sbi->s_freeclusters_counter, ac->ac_b_ex.fe_len);
> > -	/*
> > -	 * Now reduce the dirty block count also. Should not go negative
> > -	 */
> > -	if (!(ac->ac_flags & EXT4_MB_DELALLOC_RESERVED))
> > -		/* release all the reserved blocks if non delalloc */
> > -		percpu_counter_sub(&sbi->s_dirtyclusters_counter,
> > -				   reserv_clstrs);
> >  
> >  	return err;
> >  }
> > @@ -6332,7 +6324,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
> >  			ext4_mb_pa_put_free(ac);
> >  	}
> >  	if (likely(ac->ac_status == AC_STATUS_FOUND)) {
> > -		*errp = ext4_mb_mark_diskspace_used(ac, handle, reserv_clstrs);
> > +		*errp = ext4_mb_mark_diskspace_used(ac, handle);
> >  		if (*errp) {
> >  			ext4_discard_allocated_blocks(ac);
> >  			goto errout;
> > @@ -6363,12 +6355,9 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
> >  out:
> >  	if (inquota && ar->len < inquota)
> >  		dquot_free_block(ar->inode, EXT4_C2B(sbi, inquota - ar->len));
> > -	if (!ar->len) {
> > -		if ((ar->flags & EXT4_MB_DELALLOC_RESERVED) == 0)
> > -			/* release all the reserved blocks if non delalloc */
> > -			percpu_counter_sub(&sbi->s_dirtyclusters_counter,
> > -						reserv_clstrs);
> > -	}
> > +	/* release all the reserved blocks if non delalloc */
> > +	if ((ar->flags & EXT4_MB_DELALLOC_RESERVED) == 0)
> 
> Nit: It might be better to check if (reserv_clstrs) directly. It’s more
> straightforward and stays robust even if the flag logic changes later.

Sure.

Brian

> 
> > +		percpu_counter_sub(&sbi->s_dirtyclusters_counter, reserv_clstrs);
> >  
> >  	trace_ext4_allocate_blocks(ar, (unsigned long long)block);
> >  
> 
>