Re: [PATCH] btrfs: prevent COW amplification during btrfs_search_slot

[Sun YangKai <sunk67188@gmail.com>]

On 2026/1/30 17:37, Sun YangKai wrote:
> 
> 
> On 2026/1/30 12:14, Sun YangKai wrote:
>> On 2026/1/30 08:12, Leo Martins wrote:
>>> On Thu, 29 Jan 2026 11:52:07 +0000 Filipe Manana<fdmanana@kernel.org> 
>>> wrote:
>>>> On Tue, Jan 27, 2026 at 8:43 PM Leo Martins<loemra.dev@gmail.com> 
>>>> wrote:
>>>>> I've been investigating enospcs at Meta and have observed a strange
>>>>> pattern where filesystems are enospcing with lots of unallocated space
>>>>> (> 100G). Sample dmesg dump at bottom of message.
>>>>>
>>>>> btrfs_insert_delayed_dir_index is attempting to migrate some 
>>>>> reservation
>>>>> from the transaction block reserve and finding it exhausted leading 
>>>>> to a
>>>>> warning and enospc. This is a bug as the reservations are meant to be
>>>>> worst case. It should be impossible to exhaust the transaction block
>>>>> reserve.
>>>>>
>>>>> Some tracing of affected hosts revealed that there were single
>>>>> btrfs_search_slot calls that were COWing 100s of times. I was able to
>>>>> reproduce this behavior locally by creating a very constrained cgroup
>>>>> and producing a lot of concurrent filesystem operations. Here's the
>>>>> pattern:
>>>>>
>>>>>   1. btrfs_search_slot() begins tree traversal with cow=1
>>>>>   2. Node at level N needs COW (old generation or WRITTEN flag set)
>>>>>   3. btrfs_cow_block() allocates new node, updates parent pointer
>>>>>   4. Traversal continues, but hits a condition requiring restart 
>>>>> (e.g., node
>>>>>      not cached, lock contention, need higher write_lock_level)
>>>>>   5. btrfs_release_path() releases all locks and references
>>>>>   6. Memory pressure triggers writeback on the COW'd node
>>>>>   7. lock_extent_buffer_for_io() clears EXTENT_BUFFER_DIRTY and sets
>>>>>      BTRFS_HEADER_FLAG_WRITTEN
>>>>>   8. goto again - traversal restarts from root
>>>>>   9. Traversal reaches the freshly COW'd node
>>>>>   10. should_cow_block() sees WRITTEN flag set, returns true
>>>>>   11. btrfs_cow_block() allocates another new node - same logical 
>>>>> position,
>>>>>       new physical location, new reservation consumed
>>>>>   12. Steps 4-11 repeat indefinitely under sustained memory pressure
>>>>>
>>>>> Note this behavior should be much harder to trigger since Boris's
>>>>> AS_KERNEL_FILE changes that make it so that extent_buffer pages aren't
>>>>> accounted for in user cgroups. However, I believe it
>>>>> would still be an issue under global memory pressure.
>>>>> Link:https://lore.kernel.org/linux-btrfs/ 
>>>>> cover.1755812945.git.boris@bur.io/
>>>>>
>>>>> This COW amplification breaks the idea that transaction 
>>>>> reservations are
>>>>> worst case as any search slot call could find itself in this COW 
>>>>> loop and
>>>>> exhaust its reservation.
>>>>>
>>>>> My proposed solution is to temporarily pin extent buffers for the
>>>>> lifetime of btrfs_search_slot. This prevents the massive COW
>>>>> amplification that can be seen during high memory pressure.
>>>>>
>>>>> The implementation uses a local xarray to track COW'd buffers for the
>>>>> duration of the search. The xarray stores extent_buffer pointers 
>>>>> without
>>>>> taking additional references; this is safe because tracked buffers 
>>>>> remain
>>>>> dirty (writeback_blockers prevents the dirty bit from being 
>>>>> cleared) and
>>>>> dirty buffers cannot be reclaimed by memory pressure.
>>>>>
>>>>> Synchronization is provided by eb->lock: increments in
>>>>> btrfs_search_slot_track_cow() occur while holding the write lock, and
>>>>> the check in lock_extent_buffer_for_io() also holds the write lock via
>>>>> btrfs_tree_lock(). Decrements don't require eb->lock because
>>>>> writeback_blockers is atomic and merely indicates "don't write yet".
>>>>> Once we decrement, we're done and don't care if writeback proceeds
>>>>> immediately.
>>>> This seems too complex to me.
>>>>
>>>> So this problem is very similar to some idea I had a few years ago but
>>>> never managed to implement.
>>>> It was about avoiding unnecessary COW, not for this space reservation
>>>> exhaustion due to sustained memory pressure, but it would solve it
>>>> too.
>>>>
>>>> The idea was that we do unnecessary COW in cases like this:
>>>>
>>>> 1) We COW a path in some tree and we are at transaction N;
>>>>
>>>> 2) Writeback happened for the extent buffers in that path while we are
>>>> in the same transaction, because we reached the 32M limit and some
>>>> task called btrfs_btree_balance_dirty() or something else triggered
>>>> writeback of the btree inode;
>>>>
>>>> 3) While still at transaction N, we visit the same path to add an item
>>>> to a leaf, or modify an item, whatever. Because the extent buffers
>>>> have BTRFS_HEADER_FLAG_WRITTEN, we COW them again (should_cow_block()
>>>> returns true).
>>>>
>>>> So during the lifetime of a transaction we can have a lot of
>>>> unnecessary COW - we spend more time allocating extents, allocating
>>>> memory, copying extent buffer data, use more space per transaction,
>>>> etc.
>>>>
>>>> The idea was to not COW when an extent buffer has
>>>> BTRFS_HEADER_FLAG_WRITTEN set, but only if its generation
>>>> (btrfs_header_generation(eb)) matches the current transaction.
>>>> That is safe because there's no committed tree that points to an
>>>> extent buffer created in the current transaction.
>>>>
>>>> Any further modification to the extent buffer must be sure that the
>>>> EXTENT_BUFFER_DIRTY flag is set, that the eb range is still in the
>>>> transaction's dirty_pages io tree, etc, so that we don't miss writing
>>>> the extent buffer to the same location again before the transaction
>>>> commits the superblocks.
>>>>
>>>> Have you considered an approach like this?
>>> I had not considered this, but it is a great idea.
>>>
>>> My first thought is that implementing this could be as simple
>>> as removing the BTRFS_HEADER_FLAG_WRITTEN check. However, this
>>> would mess with the assumptions around the log tree. From
>>> btrfs_sync_log():
>> After a fast glance and some tests, I found things might not be that 
>> easy. The problem is not only the log tree.
>>> /*
>>>   * IO has been started, blocks of the log tree have WRITTEN flag set
>>>   * in their headers. new modifications of the log will be written to
>>>   * new positions. so it's safe to allow log writers to go in.
>>>   */
>>>
>>> ^ Assumes that WRITTEN blocks will be COW'd.
>>>
>>> The issue looks like:
>>>
>>>   1. fsync A COWs eb
>>>   2. fsync A lock_extent_buffer_for_io(); sets WRITTEN, unlocks tree
>>>   3. fsync B does __not__ COW eb and modifies it
>>>   4. fsync A writes modified eb to disk
>>>   5. CRASH; the log tree is corrupted
>>>
>>> One way to avoid that is to keep the current behavior for the log
>>> tree, but that leaves the potential for COW amplification...
>> I tested with a patch like this:
>> @@ -624,14 +624,18 @@ static inline bool should_cow_block(const struct 
>> btrfs_trans_handle *trans,
>>          if (btrfs_header_generation(buf) != trans->transid)
>>                  return true;
>>
>> -       if (btrfs_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN))
>> -               return true;
>> -
>>          /* Ensure we can see the FORCE_COW bit. */
>>          smp_mb__before_atomic();
>>          if (test_bit(BTRFS_ROOT_FORCE_COW, &root->state))
>>                  return true;
>>
>> +       if (btrfs_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN)) {
>> +               if (btrfs_root_id(root) == BTRFS_TREE_LOG_OBJECTID)
>> +                       return true;
>> +               btrfs_mark_buffer_dirty(trans, buf);
>> +               return false;
>> +       }
>> +
>>          if (btrfs_root_id(root) == BTRFS_TREE_RELOC_OBJECTID)
>>
>>                  return false;
>>
>> And get some errors like this:
>>
>>
>> [  +0.090163] [ T2589] run fstests btrfs/004 at 2026-01-30 11:53:37
>> [  +0.432352] [T11685] BTRFS: device fsid 1fb397fc-97a7-44dd-9602- 
>> dd38b74bc391 devid 1 transid 8 /dev/loop1 (7:1) scanned by mount (11685)
>> [  +0.000351] [T11685] BTRFS info (device loop1): first mount of 
>> filesystem 1fb397fc-97a7-44dd-9602-dd38b74bc391
>> [  +0.000014] [T11685] BTRFS info (device loop1): using crc32c 
>> (crc32c- lib) checksum algorithm
>> [  +0.001298] [T11685] BTRFS info (device loop1): checking UUID tree
>> [  +0.000039] [T11685] BTRFS info (device loop1): enabling ssd 
>> optimizations
>> [  +0.000003] [T11685] BTRFS info (device loop1): turning on async 
>> discard
>> [  +0.000002] [T11685] BTRFS info (device loop1): enabling free space 
>> tree
>> [  +1.051781] [T11703] page: refcount:2 mapcount:0 
>> mapping:00000000eb6d7caa index:0x2348 pfn:0x1caebf
>> [  +0.000008] [T11703] memcg:ffff9b3300263cc0
>> [  +0.000003] [T11703] aops:0xffffffffc0354040 ino:1
>> [  +0.000024] [T11703] flags: 0x4e0000000000423e(referenced|uptodate| 
>> dirty|lru|workingset|private|writeback|zone=1)
>> [  +0.000007] [T11703] raw: 4e0000000000423e fffff74a872bb908 
>> fffff74a84206a88 ffff9b33c6706880
>> [  +0.000004] [T11703] raw: 0000000000002348 ffff9b334be522d0 
>> 00000002ffffffff ffff9b3300263cc0
>> [  +0.000002] [T11703] page dumped because: eb page dump
>> [  +0.000003] [T11703] BTRFS critical (device loop1): corrupt leaf: 
>> root=5 block=36995072 slot=118 ino=406 file_offset=94208, invalid 
>> ram_bytes for file extent, have 8660273067269322872, should be aligned 
>> to 4096
>> [  +0.000013] [T11703] BTRFS info (device loop1): leaf 36995072 gen 33 
>> total ptrs 128 free space 2857 owner 5
>> [  +0.000006] [T11703]     item 0 key (386 DIR_ITEM 238230307) itemoff 
>> 16249 itemsize 34
>> [  +0.000004] [T11703]         location key (462 1 0) type 2
>> [  +0.000003] [T11703]         transid 33 data_len 0 name_len 4
>> [  +0.000003] [T11703]     item 1 key (386 DIR_ITEM 1473745676) 
>> itemoff 16216 itemsize 33
>> [  +0.000004] [T11703]         location key (376 1 0) type 3
>> [  +0.000002] [T11703]         transid 30 data_len 0 name_len 3
>> [  +0.000003] [T11703]     item 2 key (386 DIR_ITEM 2243137595) 
>> itemoff 16182 itemsize 34
>> [  +0.000004] [T11703]         location key (413 1 0) type 1
>> [  +0.000002] [T11703]         transid 32 data_len 0 name_len 4
>> ...
>> [  +0.000001] [T11703]     item 127 key (405 DIR_ITEM 828387202) 
>> itemoff 6057 itemsize 34
>> [  +0.000002] [T11703]         location key (479 1 0) type 3
>> [  +0.000001] [T11703]         transid 33 data_len 0 name_len 4
>> [  +0.000002] [T11703] BTRFS error (device loop1): block=36995072 
>> write time tree block corruption detected
>> [  +0.003429] [T11703] BTRFS: error (device loop1) in 
>> btrfs_commit_transaction:2555: errno=-5 IO failure (Error while 
>> writing out transaction)
>> [  +0.000007] [T11703] BTRFS info (device loop1 state E): forced readonly
>> [  +0.000002] [T11703] BTRFS warning (device loop1 state E): Skipping 
>> commit of aborted transaction.
>> [  +0.000002] [T11703] BTRFS error (device loop1 state EA): 
>> Transaction aborted (error -5)
>> [  +0.000003] [T11703] BTRFS: error (device loop1 state EA) in 
>> cleanup_transaction:2037: errno=-5 IO failure
>>
>> The reported 406 inode is even not in the printed leaf. It seems like 
>> a data race maybe caused by:
>>
>> We unlock the eb after setting the WRITTEN flag during write back, and 
>> the eb should not get modified since then because all future writes 
>> will use the cowed eb. However, with the WRITTEN flag check removed in 
>> should_cow_block, we might write to the eb with WRITTEN flag set which 
>> might be under io.
> 
> I tried again with this:
> 
> @@ -624,14 +624,20 @@ static inline bool should_cow_block(const struct 
> btrfs_trans_handle *trans,
>          if (btrfs_header_generation(buf) != trans->transid)
>                  return true;
> 
> -       if (btrfs_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN))
> -               return true;
> -
>          /* Ensure we can see the FORCE_COW bit. */
>          smp_mb__before_atomic();
>          if (test_bit(BTRFS_ROOT_FORCE_COW, &root->state))
>                  return true;
> 
> +       if (btrfs_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN)) {
> +               if (btrfs_root_id(root) == BTRFS_TREE_LOG_OBJECTID)
> +                       return true;
> +               if (test_bit(EXTENT_BUFFER_WRITEBACK, &buf->bflags))
> +                       return true;
> +               btrfs_mark_buffer_dirty(trans, buf);
> +               return false;
> +       }
> +
>          if (btrfs_root_id(root) == BTRFS_TREE_RELOC_OBJECTID)
>                  return false;
> 
> When WRITEBACK is set, do a normal cow to prevent the data race. This 
> seems to fix the previous problem. However, I got this:
> 
> [  +0.020843] [T15127] BTRFS error (device loop1): block=30687232 bad 
> generation, have 11 expect > 14
> [  +0.000009] [T15127]     item 0 key (256 INODE_ITEM 0) itemoff 16123 
> itemsize 160
> [  +0.000004] [T15127]         inode generation 3 transid 11 size 10 
> nbytes 16384
> [  +0.000003] [T15127]         block group 0 mode 40755 links 1 uid 0 gid 0
> [  +0.000002] [T15127]         rdev 0 sequence 1 flags 0x0
> [  +0.000002] [T15127]         atime 1769760651.0
> [  +0.000002] [T15127]         ctime 1769760652.250234845
> [  +0.000002] [T15127]         mtime 1769760652.250234845
> [  +0.000001] [T15127]         otime 1769760651.0
> ...
> [  +0.000004] [T15127]         root data bytenr 30523392 refs 1
> [  +0.000002] [T15127] BTRFS error (device loop1): block=30851072 write 
> time tree block corruption detected
> 
> and a lot more lines with the same generation errors for btrfs/122 
> btrfs/152 btrfs/210 btrfs/224 btrfs/316 btrfs/320 btrfs/340 fstest cases.
> 
> I have no idea why it's trying to write some ebs older than current 
> transaction. Seems related with snapshots.

This happens because after an extent buffer (eb) is written to disk, 
subsequent modifications only set the dirty flag without adding those 
pages to the current transaction's dirty list. Consequently, their 
writeback isn't triggered or awaited during transaction commit.

In contrast, newly allocated or COWed extent buffers are explicitly 
added to the transaction's dirty_pages via btrfs_init_new_buffer, which 
ensures they are properly tracked and written back.

Add the following code could fix this:

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 8d683745afd1..3ab89a31f9bb 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4450,6 +4450,9 @@ void btrfs_mark_buffer_dirty(struct 
btrfs_trans_handle *trans,
                            buf->start, transid, fs_info->generation);
         }
         set_extent_buffer_dirty(buf);
+       if (btrfs_header_owner(buf) != BTRFS_TREE_LOG_OBJECTID)
+               btrfs_set_extent_bit(&trans->transaction->dirty_pages, 
buf->start,
+                                    buf->start + buf->len - 1, 
EXTENT_DIRTY, NULL);
  }

  static void __btrfs_btree_balance_dirty(struct btrfs_fs_info *fs_info,


Thanks,
Sun YangKai