From: Zygo Blaxell Date: Wed, 16 Sep 2026 04:47:31 +0000 (-0400) Subject: btrfs: stripe_alloc: charge the preallocation's stripe hold under the reservation... X-Git-Url: http://git.hungrycats.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b757886dbdbd0c2f402a716a0a3c959504000df5;p=linux btrfs: stripe_alloc: charge the preallocation's stripe hold under the reservation lock A preallocation is admitted for round_up(bytes, stripe) against the whole-stripe supply and then holds one stripe in bytes_stripe_margin until its allocations land, but the hold was added by the caller after btrfs_alloc_data_chunk_ondemand() returned: __reserve_bytes() recorded only the raw bytes in bytes_may_use and dropped space_info->lock in between. Two sub-stripe preallocations admitted in that window each saw the other's raw bytes only, both passed against one stripe of supply, and a buffered write admitted alongside them could find no stripe at writeback. The delalloc path does not have this gap (its margin is probed into bytes_may_use with the bytes and kept there); make the preallocation path the same. Charge the hold inside __reserve_bytes(), in the critical section that grants the bytes -- for a direct admission and for a ticket granted later by btrfs_try_granting_tickets() -- and hand it back through a new btrfs_alloc_data_chunk_ondemand_held() / btrfs_reserve_data_bytes_held() to the four preallocating callers (fallocate's range loop, zero-range, relocation's cluster preallocation, the encoded write). The release is unchanged. Found by review (2026-09-15); prealloc-race-test.sh (eight fallocate() storms against buffered writers at the fill edge) did not reproduce the window in six rounds before the change and stays clean after it. Assisted-by: Claude:claude-fable-5-1 --- diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c index 54cd0a97f1edb..835c144242f68 100644 --- a/fs/btrfs/delalloc-space.c +++ b/fs/btrfs/delalloc-space.c @@ -124,6 +124,17 @@ static inline struct btrfs_space_info *data_sinfo_for_inode(const struct btrfs_i } int btrfs_alloc_data_chunk_ondemand(const struct btrfs_inode *inode, u64 bytes) +{ + return btrfs_alloc_data_chunk_ondemand_held(inode, bytes, NULL); +} + +/* + * With @stripe_held: a preallocation, which claims whole stripes under + * stripe_alloc; the reservation charges one stripe of collateral together + * with the bytes and returns it for btrfs_stripe_prealloc_release(). + */ +int btrfs_alloc_data_chunk_ondemand_held(const struct btrfs_inode *inode, + u64 bytes, u64 *stripe_held) { struct btrfs_root *root = inode->root; struct btrfs_fs_info *fs_info = root->fs_info; @@ -139,7 +150,8 @@ int btrfs_alloc_data_chunk_ondemand(const struct btrfs_inode *inode, u64 bytes) if (btrfs_is_free_space_inode(inode)) flush = BTRFS_RESERVE_FLUSH_FREE_SPACE_INODE; - return btrfs_reserve_data_bytes(data_sinfo_for_inode(inode), bytes, flush); + return btrfs_reserve_data_bytes_held(data_sinfo_for_inode(inode), bytes, + flush, stripe_held); } int btrfs_check_data_free_space(struct btrfs_inode *inode, diff --git a/fs/btrfs/delalloc-space.h b/fs/btrfs/delalloc-space.h index c464cf0a88d9b..37860ced7a723 100644 --- a/fs/btrfs/delalloc-space.h +++ b/fs/btrfs/delalloc-space.h @@ -10,6 +10,8 @@ struct btrfs_inode; struct btrfs_fs_info; int btrfs_alloc_data_chunk_ondemand(const struct btrfs_inode *inode, u64 bytes); +int btrfs_alloc_data_chunk_ondemand_held(const struct btrfs_inode *inode, + u64 bytes, u64 *stripe_held); int btrfs_check_data_free_space(struct btrfs_inode *inode, struct extent_changeset **reserved, u64 start, u64 len, bool noflush, u64 *stripe_held); diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c index 7204cc09d2489..f18f47f1b9083 100644 --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -3066,12 +3066,12 @@ reserve_space: const u64 lockend = alloc_end - 1; bytes_to_reserve = alloc_end - alloc_start; - ret = btrfs_alloc_data_chunk_ondemand(BTRFS_I(inode), - bytes_to_reserve); + ret = btrfs_alloc_data_chunk_ondemand_held(BTRFS_I(inode), + bytes_to_reserve, + &stripe_held); if (ret < 0) goto out; space_reserved = true; - stripe_held = btrfs_stripe_prealloc_hold(fs_info); btrfs_punch_hole_lock_range(inode, lockstart, lockend, &cached_state); ret = btrfs_qgroup_reserve_data(BTRFS_I(inode), &data_reserved, @@ -3246,12 +3246,11 @@ static long btrfs_fallocate(struct file *file, int mode, * We are safe to reserve space here as we can't have delalloc * in the range, see above. */ - ret = btrfs_alloc_data_chunk_ondemand(BTRFS_I(inode), - data_space_needed); - if (!ret) { + ret = btrfs_alloc_data_chunk_ondemand_held(BTRFS_I(inode), + data_space_needed, + &stripe_held); + if (!ret) data_space_reserved = data_space_needed; - stripe_held = btrfs_stripe_prealloc_hold(inode_to_fs_info(inode)); - } } /* diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 9c9011f20c441..d1652a9019f1b 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -10032,10 +10032,10 @@ ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from, * We don't use the higher-level delalloc space functions because our * num_bytes and disk_num_bytes are different. */ - ret = btrfs_alloc_data_chunk_ondemand(inode, disk_num_bytes); + ret = btrfs_alloc_data_chunk_ondemand_held(inode, disk_num_bytes, + &stripe_held); if (ret) goto out_unlock; - stripe_held = btrfs_stripe_prealloc_hold(fs_info); ret = btrfs_qgroup_reserve_data(inode, &data_reserved, start, num_bytes); if (ret) goto out_free_data_space; diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index d0344fc7016cc..8fbed5ec8d988 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -2701,11 +2701,11 @@ static noinline_for_stack int prealloc_file_extent_cluster(struct reloc_control return ret; BUG_ON(cluster->start != cluster->boundary[0]); - ret = btrfs_alloc_data_chunk_ondemand(inode, - prealloc_end + 1 - prealloc_start); + ret = btrfs_alloc_data_chunk_ondemand_held(inode, + prealloc_end + 1 - prealloc_start, + &stripe_held); if (ret) return ret; - stripe_held = btrfs_stripe_prealloc_hold(inode->root->fs_info); btrfs_inode_lock(inode, 0); for (nr = 0; nr < cluster->nr; nr++) { diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c index 0f9aad5667bc3..2e9af7667110e 100644 --- a/fs/btrfs/space-info.c +++ b/fs/btrfs/space-info.c @@ -471,16 +471,26 @@ void btrfs_stripe_margin_mod(struct btrfs_inode *inode, int mod) * caller's allocations are done. Returns the bytes held, 0 when the gate * is not in effect; the caller hands it back to the release. */ -u64 btrfs_stripe_prealloc_hold(struct btrfs_fs_info *fs_info) +/* + * Charge one whole stripe of collateral for a preallocation, under + * space_info->lock and together with the bytes it is admitted for: the + * caller's allocations claim round_up(bytes, stripe) whole stripes, and the + * hold keeps that stripe out of the supply the gate shows to everyone else + * until the claims land (btrfs_stripe_prealloc_release()). Charged after + * the admission, with the lock dropped in between, two sub-stripe + * preallocations could both be admitted against one stripe of supply and a + * buffered write admitted alongside them found no stripe at writeback. + */ +static u64 stripe_hold_locked(struct btrfs_space_info *space_info) { - struct btrfs_space_info *sinfo = fs_info->data_sinfo; + struct btrfs_fs_info *fs_info = space_info->fs_info; const u64 unit = READ_ONCE(fs_info->stripe_margin_unit); - if (!unit || !sinfo || !btrfs_test_opt(fs_info, STRIPE_ALLOC)) + lockdep_assert_held(&space_info->lock); + if (!unit || !(space_info->flags & BTRFS_BLOCK_GROUP_DATA) || + !btrfs_test_opt(fs_info, STRIPE_ALLOC)) return 0; - spin_lock(&sinfo->lock); - sinfo->bytes_stripe_margin += unit; - spin_unlock(&sinfo->lock); + space_info->bytes_stripe_margin += unit; return unit; } @@ -754,6 +764,8 @@ again: flush, ticket->reloc)) && stripe_claimable_admit(space_info, ticket->bytes, false)) { btrfs_space_info_update_bytes_may_use(space_info, ticket->bytes); + if (ticket->hold_stripe) + ticket->held = stripe_hold_locked(space_info); remove_ticket(space_info, ticket); ticket->bytes = 0; space_info->tickets_id++; @@ -1970,7 +1982,7 @@ static inline bool can_ticket(enum btrfs_reserve_flush_enum flush) */ static int __reserve_bytes(struct btrfs_fs_info *fs_info, struct btrfs_space_info *space_info, u64 orig_bytes, - enum btrfs_reserve_flush_enum flush) + enum btrfs_reserve_flush_enum flush, u64 *held) { struct work_struct *async_work; struct reserve_ticket ticket; @@ -2039,6 +2051,8 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info, stripe_claimable_admit(space_info, orig_bytes, flush == BTRFS_RESERVE_FLUSH_DATA_RELOC)) { btrfs_space_info_update_bytes_may_use(space_info, orig_bytes); + if (held) + *held = stripe_hold_locked(space_info); ret = 0; } @@ -2069,6 +2083,8 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info, init_waitqueue_head(&ticket.wait); ticket.steal = can_steal(flush); ticket.reloc = reloc; + ticket.hold_stripe = held != NULL; + ticket.held = 0; if (trace_btrfs_reserve_ticket_enabled()) start_ns = ktime_get_ns(); @@ -2116,8 +2132,11 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info, if (!ret || !can_ticket(flush)) return ret; - return handle_reserve_ticket(fs_info, space_info, &ticket, start_ns, - orig_bytes, flush); + ret = handle_reserve_ticket(fs_info, space_info, &ticket, start_ns, + orig_bytes, flush); + if (!ret && held) + *held = ticket.held; + return ret; } /* @@ -2142,7 +2161,7 @@ int btrfs_reserve_metadata_bytes(struct btrfs_fs_info *fs_info, { int ret; - ret = __reserve_bytes(fs_info, space_info, orig_bytes, flush); + ret = __reserve_bytes(fs_info, space_info, orig_bytes, flush, NULL); if (ret == -ENOSPC) { trace_btrfs_space_reservation(fs_info, "space_info:enospc", space_info->flags, orig_bytes, 1); @@ -2165,6 +2184,18 @@ int btrfs_reserve_metadata_bytes(struct btrfs_fs_info *fs_info, */ int btrfs_reserve_data_bytes(struct btrfs_space_info *space_info, u64 bytes, enum btrfs_reserve_flush_enum flush) +{ + return btrfs_reserve_data_bytes_held(space_info, bytes, flush, NULL); +} + +/* + * Like btrfs_reserve_data_bytes(), for a preallocation: with @held the + * reservation also charges one whole stripe of stripe_alloc collateral, + * atomically with the bytes, and returns it for + * btrfs_stripe_prealloc_release() once the caller's allocations are done. + */ +int btrfs_reserve_data_bytes_held(struct btrfs_space_info *space_info, u64 bytes, + enum btrfs_reserve_flush_enum flush, u64 *held) { struct btrfs_fs_info *fs_info = space_info->fs_info; int ret; @@ -2177,7 +2208,9 @@ int btrfs_reserve_data_bytes(struct btrfs_space_info *space_info, u64 bytes, (flush != BTRFS_RESERVE_FLUSH_DATA && flush != BTRFS_RESERVE_FLUSH_DATA_RELOC)); - ret = __reserve_bytes(fs_info, space_info, bytes, flush); + if (held) + *held = 0; + ret = __reserve_bytes(fs_info, space_info, bytes, flush, held); if (ret == -ENOSPC) { trace_btrfs_space_reservation(fs_info, "space_info:enospc", space_info->flags, bytes, 1); diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h index 7065ee6daddea..7b21f3f70e71b 100644 --- a/fs/btrfs/space-info.h +++ b/fs/btrfs/space-info.h @@ -296,6 +296,13 @@ struct reserve_ticket { bool steal; /* Made by the relocation task: may draw on bytes_stripe_reserve. */ bool reloc; + /* + * A preallocation: one whole stripe of collateral is charged into + * bytes_stripe_margin in the same critical section that grants the + * bytes (stripe_hold_locked()), and handed back through @held. + */ + bool hold_stripe; + u64 held; struct list_head list; wait_queue_head_t wait; }; @@ -341,7 +348,8 @@ struct btrfs_inode; int btrfs_init_space_info(struct btrfs_fs_info *fs_info); void btrfs_stripe_margin_mod(struct btrfs_inode *inode, int mod); -u64 btrfs_stripe_prealloc_hold(struct btrfs_fs_info *fs_info); +int btrfs_reserve_data_bytes_held(struct btrfs_space_info *space_info, u64 bytes, + enum btrfs_reserve_flush_enum flush, u64 *held); void btrfs_stripe_prealloc_release(struct btrfs_fs_info *fs_info, u64 held); void btrfs_add_bg_to_space_info(struct btrfs_fs_info *info, struct btrfs_block_group *block_group);