From: Zygo Blaxell Date: Sun, 16 Aug 2026 20:26:54 +0000 (-0400) Subject: btrfs: relocation: reserve all of a folio's slices before dirtying any X-Git-Url: http://git.hungrycats.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fheads%2Fmisc-next%2Ftopics%2Fdeadlock-fixes;p=linux btrfs: relocation: reserve all of a folio's slices before dirtying any relocate_one_folio() locks the target folio and then, for each cluster extent slice inside it, takes a flushing delalloc metadata reservation, marks the slice delalloc and re-dirties it. From the second slice on, the flushing reservation runs while we hold a locked, partially dirty folio: under pressure it sleeps waiting for a reservation ticket, and the flush states that ticket depends on -- FLUSH_DELALLOC writing back the relocation inode (the data reloc root sits on fs_info->delalloc_roots like any other, and BTRFS_INODE_NO_DELALLOC_FLUSH is only honoured under in_reclaim_context), or a transaction commit, which with flushoncommit waits for the same writeback -- block in __folio_lock() on the folio we hold. Note the extent lock is released at the end of every slice iteration, so the folio lock is the resource still held across the next reservation. Relocation wedges the filesystem in exactly the situation (space pressure) that balance is usually run to relieve. Reproduced on a plain single-device filesystem (mkfs.btrfs -d single -m single, mounted -o noatime,max_inline=0,flushoncommit) by filling a block group with 4K extents and running balance against concurrent rewrites, snapshots and fsstress under space pressure. In the capture below the task blocked on the folio is the async reclaim worker itself -- the worker whose job is to serve the ticket the balance task is waiting for -- so the cycle closes with no third party and without needing flushoncommit at all; flush_space()'s FLUSH_DELALLOC state alone suffices. Commits were frozen at 5519: == 2439077 (btrfs) -- balance, blocked on the ticket == reserve_bytes+0x612/0xa00 btrfs_reserve_metadata_bytes+0x21/0x120 btrfs_delalloc_reserve_metadata+0x134/0x330 relocate_file_extent_cluster+0x3f5/0x8f0 <- relocate_one_folio inlined relocate_data_extent.constprop.0+0x15e/0x190 relocate_block_group+0x448/0x610 btrfs_relocate_block_group+0xa9f/0x2710 btrfs_relocate_chunk+0x52/0x200 btrfs_balance+0xd6d/0x1f30 btrfs_ioctl+0x185c/0x2a60 __x64_sys_ioctl+0xa5/0x100 == 1759399 (kworker/u32:11+events_unbound) -- must serve that ticket == folio_wait_bit_common+0x177/0x3a0 __folio_lock+0x17/0x30 lock_delalloc_folios+0x1cc/0x2d0 find_lock_delalloc_range+0x148/0x2a0 writepage_delalloc+0x29c/0x980 extent_write_cache_pages+0x333/0xc50 btrfs_writepages+0x67/0xd0 do_writepages+0xb7/0x180 filemap_writeback+0xd1/0x100 filemap_flush_nr+0x1f/0x30 start_delalloc_inodes+0xd3/0x400 btrfs_start_delalloc_roots+0x184/0x270 flush_space+0x3a8/0x690 do_async_reclaim_metadata_space+0x92/0x170 btrfs_async_reclaim_metadata_space+0x64/0x90 A second capture of the same run shows the longer route, via a wb_workfn writeback thread in __folio_lock() under extent_write_cache_pages() with the commit waiting on it. One slice per folio -- the 4K page, 4K sectorsize, order-0 case -- cannot deadlock: the single reservation happens while the folio is still clean, and writeback has no business with a clean folio. Multiple slices per folio need folio_size > extent size. That was originally only reachable with sub-page sector sizes, but 041c39da53c2 ("btrfs: enable large data folios for data reloc inode") made it reachable on 4K pages under CONFIG_BTRFS_EXPERIMENTAL, and 9bce95edb1b4 ("btrfs: move large data folios out of experimental features") makes it reachable in default builds: on 4K sectors calc_block_max_order() yields order 6, so this loop can be handed a 256K folio spanning up to 64 slices. Restructure the function to take all of the folio's slice reservations up front, while the folio is still clean, keeping the per-slice reservation granularity (each slice becomes its own delalloc extent because of EXTENT_BOUNDARY, so per-slice accounting is the accurate form). A clean folio carries no dirty tag, so writeback never returns it and never asks for its lock, and its range has no EXTENT_DELALLOC yet, so find_lock_delalloc_range() cannot reach it either; flushing there is safe. The dirtying loop then consumes the reservations without ever flushing under a dirty folio, and unwinding on error walks the same slice geometry so reserve/release pair exactly. ENOSPC semantics are unchanged: the up-front reservations flush exactly like the old in-loop ones, just at a point where flushing can still make progress. This relies on prealloc_file_extent_cluster() calling filemap_invalidate_inode() with flush=true for the cluster range: a folio shared with the previous cluster is written back and dropped there, so it is genuinely clean when we reserve for it here. The deadlock is a race -- the reservation has to ticket while relocation is between slices of one folio -- so single runs prove little in either direction. Over repeated 15-minute runs of the above workload, three viable runs on the unfixed kernel deadlocked twice, while three on the fixed kernel completed with no occurrence, and with the relocating task still observed blocked on a metadata reservation ticket and no writeback task ever blocked on a folio: the reservation still tickets and still flushes, it just no longer traps the flusher. Fixes: c2832898126f ("btrfs: make relocate_one_page() handle subpage case") Assisted-by: Claude:claude-fable-5 --- diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c index 630a7ad8f8e1a..cfa14fb1e5dbf 100644 --- a/fs/btrfs/relocation.c +++ b/fs/btrfs/relocation.c @@ -2900,6 +2900,30 @@ static u64 get_cluster_boundary_end(const struct file_extent_cluster *cluster, return cluster->boundary[cluster_nr + 1] - 1; } +/* + * Release the per-slice delalloc metadata reservations taken up front by + * relocate_one_folio() for the cluster extent slices covering [cur, end]. + * @nr is the cluster extent index of the slice containing @cur. Walks the + * same slice geometry as the reservation pass so the releases pair exactly. + */ +static void release_cluster_slice_reservations(struct btrfs_inode *inode, + const struct file_extent_cluster *cluster, + u64 offset, int nr, u64 cur, + u64 end) +{ + while (cur <= end && nr < cluster->nr) { + u64 extent_end = get_cluster_boundary_end(cluster, nr) - offset; + u64 clamped_end = min(end, extent_end); + u32 len = clamped_end + 1 - cur; + + btrfs_delalloc_release_metadata(inode, len, true); + btrfs_delalloc_release_extents(inode, len); + cur += len; + if (cur >= extent_end) + nr++; + } +} + static int relocate_one_folio(struct reloc_control *rc, struct file_ra_state *ra, int *cluster_nr, u64 *file_offset_ret) @@ -2916,6 +2940,7 @@ static int relocate_one_folio(struct reloc_control *rc, u64 folio_start; u64 folio_end; u64 cur; + u64 rsv_end = 0; int ret; const bool use_rst = btrfs_need_stripe_tree_update(fs_info, rc->block_group->flags); @@ -2978,6 +3003,47 @@ again: * inside the folio. */ cur = max(folio_start, cluster->boundary[*cluster_nr] - offset); + + /* + * Reserve the delalloc metadata for every cluster extent slice in + * this folio up front, while the folio is still clean. Once a slice + * has been dirtied, a flushing reservation for the next one can + * sleep on a ticket that is only satisfiable by writing this inode + * back (FLUSH_DELALLOC, or a transaction commit, which with + * flushoncommit waits for the same writeback) -- and writeback + * blocks in __folio_lock() on the folio we hold. A clean locked + * folio, by contrast, is invisible to writeback, so flushing is + * still safe here. With one extent slice per folio (4K pages) this + * degenerates to the old single reservation; multiple slices happen + * with large folios or sub-page sector sizes. + */ + { + int nr = *cluster_nr; + u64 rsv_cur = cur; + + while (rsv_cur <= folio_end) { + u64 extent_end = get_cluster_boundary_end(cluster, nr) - offset; + u64 clamped_end = min(folio_end, extent_end); + u32 len = clamped_end + 1 - rsv_cur; + + ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), + len, len, false); + if (ret) { + release_cluster_slice_reservations(BTRFS_I(inode), + cluster, offset, *cluster_nr, + cur, rsv_cur - 1); + goto release_folio; + } + rsv_cur += len; + if (rsv_cur >= extent_end) { + nr++; + if (nr >= cluster->nr) + break; + } + } + rsv_end = rsv_cur - 1; + } + while (cur <= folio_end) { struct extent_state *cached_state = NULL; u64 extent_start = cluster->boundary[*cluster_nr] - offset; @@ -2987,13 +3053,6 @@ again: u64 clamped_end = min(folio_end, extent_end); u32 clamped_len = clamped_end + 1 - clamped_start; - /* Reserve metadata for this range */ - ret = btrfs_delalloc_reserve_metadata(BTRFS_I(inode), - clamped_len, clamped_len, - false); - if (ret) - goto release_folio; - /* Mark the range delalloc and dirty for later writeback */ btrfs_lock_extent(&BTRFS_I(inode)->io_tree, clamped_start, clamped_end, &cached_state); @@ -3004,10 +3063,10 @@ again: clamped_start, clamped_end, EXTENT_LOCKED | EXTENT_BOUNDARY, &cached_state); - btrfs_delalloc_release_metadata(BTRFS_I(inode), - clamped_len, true); - btrfs_delalloc_release_extents(BTRFS_I(inode), - clamped_len); + /* This slice and the not-yet-dirtied remainder. */ + release_cluster_slice_reservations(BTRFS_I(inode), + cluster, offset, *cluster_nr, + clamped_start, rsv_end); goto release_folio; } btrfs_folio_set_dirty(fs_info, folio, clamped_start, clamped_len);