Skip to content

Commit 3924aea

Browse files
Brian Fostergregkh
authored andcommitted
ext4: fix dirtyclusters double decrement on fs shutdown
commit 94a8cea upstream. 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: 0087d9f ("ext4: Fix s_dirty_blocks_counter if block allocation failed with nodelalloc") Signed-off-by: Brian Foster <bfoster@redhat.com> Reviewed-by: Baokun Li <libaokun1@huawei.com> Link: https://patch.msgid.link/20260113171905.118284-1-bfoster@redhat.com Signed-off-by: Theodore Ts'o <tytso@mit.edu> Cc: stable@kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 29a07d6 commit 3924aea

2 files changed

Lines changed: 6 additions & 17 deletions

File tree

fs/ext4/mballoc-test.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,7 @@ test_mark_diskspace_used_range(struct kunit *test,
567567

568568
bitmap = mbt_ctx_bitmap(sb, TEST_GOAL_GROUP);
569569
memset(bitmap, 0, sb->s_blocksize);
570-
ret = ext4_mb_mark_diskspace_used(ac, NULL, 0);
570+
ret = ext4_mb_mark_diskspace_used(ac, NULL);
571571
KUNIT_ASSERT_EQ(test, ret, 0);
572572

573573
max = EXT4_CLUSTERS_PER_GROUP(sb);

fs/ext4/mballoc.c

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4181,8 +4181,7 @@ ext4_mb_mark_context(handle_t *handle, struct super_block *sb, bool state,
41814181
* Returns 0 if success or error code
41824182
*/
41834183
static noinline_for_stack int
4184-
ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
4185-
handle_t *handle, unsigned int reserv_clstrs)
4184+
ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac, handle_t *handle)
41864185
{
41874186
struct ext4_group_desc *gdp;
41884187
struct ext4_sb_info *sbi;
@@ -4237,13 +4236,6 @@ ext4_mb_mark_diskspace_used(struct ext4_allocation_context *ac,
42374236
BUG_ON(changed != ac->ac_b_ex.fe_len);
42384237
#endif
42394238
percpu_counter_sub(&sbi->s_freeclusters_counter, ac->ac_b_ex.fe_len);
4240-
/*
4241-
* Now reduce the dirty block count also. Should not go negative
4242-
*/
4243-
if (!(ac->ac_flags & EXT4_MB_DELALLOC_RESERVED))
4244-
/* release all the reserved blocks if non delalloc */
4245-
percpu_counter_sub(&sbi->s_dirtyclusters_counter,
4246-
reserv_clstrs);
42474239

42484240
return err;
42494241
}
@@ -6328,7 +6320,7 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
63286320
ext4_mb_pa_put_free(ac);
63296321
}
63306322
if (likely(ac->ac_status == AC_STATUS_FOUND)) {
6331-
*errp = ext4_mb_mark_diskspace_used(ac, handle, reserv_clstrs);
6323+
*errp = ext4_mb_mark_diskspace_used(ac, handle);
63326324
if (*errp) {
63336325
ext4_discard_allocated_blocks(ac);
63346326
goto errout;
@@ -6359,12 +6351,9 @@ ext4_fsblk_t ext4_mb_new_blocks(handle_t *handle,
63596351
out:
63606352
if (inquota && ar->len < inquota)
63616353
dquot_free_block(ar->inode, EXT4_C2B(sbi, inquota - ar->len));
6362-
if (!ar->len) {
6363-
if ((ar->flags & EXT4_MB_DELALLOC_RESERVED) == 0)
6364-
/* release all the reserved blocks if non delalloc */
6365-
percpu_counter_sub(&sbi->s_dirtyclusters_counter,
6366-
reserv_clstrs);
6367-
}
6354+
/* release any reserved blocks */
6355+
if (reserv_clstrs)
6356+
percpu_counter_sub(&sbi->s_dirtyclusters_counter, reserv_clstrs);
63686357

63696358
trace_ext4_allocate_blocks(ar, (unsigned long long)block);
63706359

0 commit comments

Comments
 (0)