[PATCH v2] btrfs: preallocate extent changeset before acquiring extent_io_tree lock

[PATCH v2] btrfs: preallocate extent changeset before acquiring extent_io_tree lock

[tchou]

In btrfs_clear_extent_bit_changeset(), the extent changeset ulist may need
to allocate new nodes. Currently, this can happen while holding the
extent_io_tree->lock spinlock. Although ulist_prealloc() uses GFP_NOFS to
avoid deadlock with filesystem reclaim, it's better to preallocate before
acquiring the spinlock to:

1. Avoid potential allocation failures while holding the lock
2. Be consistent with set_extent_bit() which already preallocates both
   extent state and changeset before the spinlock
3. Reduce lock contention by not doing allocations under the lock

Preallocate the changeset ulist node before acquiring the spinlock,
mirroring the pattern used for extent state preallocation.

Signed-off-by: Ting-Chang Hou <tchou@synology.com>
Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 fs/btrfs/extent-io-tree.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index 626702244809..b5ba650cdb55 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -663,6 +663,9 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
 		 */
 		prealloc = alloc_extent_state(mask);
 	}
+	/* Preallocate the extent changeset ulist node before acquiring spinlock. */
+	if (changeset)
+		extent_changeset_prealloc(changeset, mask);
 
 	spin_lock(&tree->lock);
 	if (cached_state) {
-- 
2.34.1


Disclaimer: The contents of this e-mail message and any attachments are confidential and are intended solely for addressee. The information may also be legally privileged. This transmission is sent in trust, for the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or phone and delete this message and its attachments, if any.

Re: [PATCH v2] btrfs: preallocate extent changeset before acquiring extent_io_tree lock

[Qu Wenruo]

在 2026/4/17 17:08, tchou 写道:
> In btrfs_clear_extent_bit_changeset(), the extent changeset ulist may need
> to allocate new nodes. Currently, this can happen while holding the
> extent_io_tree->lock spinlock. Although ulist_prealloc() uses GFP_NOFS to
> avoid deadlock with filesystem reclaim, it's better to preallocate before
> acquiring the spinlock to:
> 
> 1. Avoid potential allocation failures while holding the lock
> 2. Be consistent with set_extent_bit() which already preallocates both
>     extent state and changeset before the spinlock
> 3. Reduce lock contention by not doing allocations under the lock
> 
> Preallocate the changeset ulist node before acquiring the spinlock,
> mirroring the pattern used for extent state preallocation.
> 
> Signed-off-by: Ting-Chang Hou <tchou@synology.com>
> Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com>

Now pushed to for-next.

Thanks,
Qu

> ---
>   fs/btrfs/extent-io-tree.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
> index 626702244809..b5ba650cdb55 100644
> --- a/fs/btrfs/extent-io-tree.c
> +++ b/fs/btrfs/extent-io-tree.c
> @@ -663,6 +663,9 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
>   		 */
>   		prealloc = alloc_extent_state(mask);
>   	}
> +	/* Preallocate the extent changeset ulist node before acquiring spinlock. */
> +	if (changeset)
> +		extent_changeset_prealloc(changeset, mask);
>   
>   	spin_lock(&tree->lock);
>   	if (cached_state) {

Re: [PATCH v2] btrfs: preallocate extent changeset before acquiring extent_io_tree lock

[Qu Wenruo]

在 2026/4/17 18:50, Qu Wenruo 写道:
> 
> 
> 在 2026/4/17 17:08, tchou 写道:
>> In btrfs_clear_extent_bit_changeset(), the extent changeset ulist may 
>> need
>> to allocate new nodes. Currently, this can happen while holding the
>> extent_io_tree->lock spinlock. Although ulist_prealloc() uses GFP_NOFS to
>> avoid deadlock with filesystem reclaim, it's better to preallocate before
>> acquiring the spinlock to:
>>
>> 1. Avoid potential allocation failures while holding the lock
>> 2. Be consistent with set_extent_bit() which already preallocates both
>>     extent state and changeset before the spinlock
>> 3. Reduce lock contention by not doing allocations under the lock
>>
>> Preallocate the changeset ulist node before acquiring the spinlock,
>> mirroring the pattern used for extent state preallocation.
>>
>> Signed-off-by: Ting-Chang Hou <tchou@synology.com>
>> Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com>

Forgot to mention, please do not add reviewed-by tags unless the 
reviewer has explicitly provided.

Especially when you do not know which mail address the reviewer is 
really using.

You do not need to resend, I have fixed the address in the for-next branch.

Thanks,
Qu

> 
> Now pushed to for-next.
> 
> Thanks,
> Qu
> 
>> ---
>>   fs/btrfs/extent-io-tree.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
>> index 626702244809..b5ba650cdb55 100644
>> --- a/fs/btrfs/extent-io-tree.c
>> +++ b/fs/btrfs/extent-io-tree.c
>> @@ -663,6 +663,9 @@ int btrfs_clear_extent_bit_changeset(struct 
>> extent_io_tree *tree, u64 start, u64
>>            */
>>           prealloc = alloc_extent_state(mask);
>>       }
>> +    /* Preallocate the extent changeset ulist node before acquiring 
>> spinlock. */
>> +    if (changeset)
>> +        extent_changeset_prealloc(changeset, mask);
>>       spin_lock(&tree->lock);
>>       if (cached_state) {
> 
> 

Re: [PATCH v2] btrfs: preallocate extent changeset before acquiring extent_io_tree lock

[Qu Wenruo]

在 2026/4/17 17:08, tchou 写道:
> In btrfs_clear_extent_bit_changeset(), the extent changeset ulist may need
> to allocate new nodes. Currently, this can happen while holding the
> extent_io_tree->lock spinlock. Although ulist_prealloc() uses GFP_NOFS to
> avoid deadlock with filesystem reclaim, it's better to preallocate before
> acquiring the spinlock to:
> 
> 1. Avoid potential allocation failures while holding the lock
> 2. Be consistent with set_extent_bit() which already preallocates both
>     extent state and changeset before the spinlock
> 3. Reduce lock contention by not doing allocations under the lock
> 
> Preallocate the changeset ulist node before acquiring the spinlock,
> mirroring the pattern used for extent state preallocation.
> 
> Signed-off-by: Ting-Chang Hou <tchou@synology.com>
> Reviewed-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
> ---

This patch cause reliable ASSERT() triggering in btrfs/022, with the 
following call trace:

[   65.405567] BTRFS info (device dm-3): qgroup scan completed 
(inconsistency flag cleared)
[   65.644601] assertion failed: 
extent_changeset_tracks_ranges(changeset), in extent_io.h:234
[   65.646833] ------------[ cut here ]------------
[   65.648119] kernel BUG at extent_io.h:234!
[   65.649323] Oops: invalid opcode: 0000 [#1] SMP
[   65.651134] CPU: 9 UID: 0 PID: 76 Comm: kworker/u50:0 Tainted: G 
      OE       7.0.0-rc7-custom+ #366 PREEMPT(full) 
e43575492b73961c1dd795487d0d2ceacbfdec38
[   65.654784] Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
[   65.656170] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 
unknown 02/02/2022
[   65.658210] Workqueue: btrfs-flush_delalloc btrfs_work_helper [btrfs]
[   65.659988] RIP: 0010:extent_changeset_prealloc.part.0+0x1f/0x21 [btrfs]
[   65.661798] Code: 0f 0b 66 0f 1f 84 00 00 00 00 00 b9 ea 00 00 00 48 
c7 c2 b2 2a 8f c0 48 c7 c6 88 92 90 c0 48 c7 c7 b8 92 90 c0 e8 a1 67 2e 
d1 <0f> 0b 48 c7 c6 c0 b0 82 c0 48 89 0c 24 e8 cf 92 30 d1 48 8b 0c 24
[   65.666489] RSP: 0018:ffffccbbc0aeb680 EFLAGS: 00010246
[   65.667835] RAX: 000000000000004f RBX: 00000000004bffff RCX: 
0000000000000000
[   65.669660] RDX: 0000000000000000 RSI: 0000000000000001 RDI: 
ffff8aea77c9d3c0
[   65.671483] RBP: 0000000000001000 R08: 0000000000000000 R09: 
00000000ffffefff
[   65.673317] R10: ffffffff93a59b00 R11: ffffccbbc0aeb520 R12: 
ffff8ae9151f6c30
[   65.675209] R13: 0000000000000000 R14: 00000000004c0000 R15: 
00000000004bffff
[   65.677076] FS:  0000000000000000(0000) GS:ffff8aeae385b000(0000) 
knlGS:0000000000000000
[   65.679132] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   65.680624] CR2: 00007f1afd800000 CR3: 0000000151124000 CR4: 
0000000000750ef0
[   65.682507] PKRU: 55555554
[   65.683259] Call Trace:
[   65.683939]  <TASK>
[   65.684525]  btrfs_clear_extent_bit_changeset.cold+0x109/0x14c [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.687187]  __btrfs_qgroup_release_data+0xa2/0x1f0 [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.689565]  alloc_ordered_extent+0x266/0x330 [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.691840]  btrfs_alloc_ordered_extent+0x290/0x350 [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.694245]  cow_file_range+0x226/0x8a0 [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.696388]  writepage_delalloc+0x35b/0x6e0 [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.698636]  extent_write_cache_pages+0x25f/0x820 [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.701004]  ? __ext4_journal_get_write_access+0x53/0x160 [ext4 
69c3e1469031ed506d5e673d78e2ebe810d3edeb]
[   65.703527]  btrfs_writepages+0x74/0x130 [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.705705]  do_writepages+0xd0/0x160
[   65.706729]  ? _raw_spin_unlock+0xe/0x20
[   65.707785]  ? wbc_attach_and_unlock_inode+0xd1/0x130
[   65.709133]  filemap_writeback+0xb5/0xe0
[   65.710192]  btrfs_run_delalloc_work+0x1b/0x50 [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.712508]  btrfs_work_helper+0xd7/0x3b0 [btrfs 
344d2e6beaedee2ce747d6035131ee8e936844c5]
[   65.714708]  process_one_work+0x198/0x3b0
[   65.715771]  worker_thread+0x1c8/0x330
[   65.716929]  ? rescuer_thread+0x5d0/0x5d0
[   65.718038]  kthread+0xee/0x120
[   65.718877]  ? kthreads_online_cpu+0x10/0x10
[   65.719998]  ret_from_fork+0x2a6/0x330
[   65.720987]  ? kthreads_online_cpu+0x10/0x10
[   65.722143]  ret_from_fork_asm+0x11/0x20
[   65.723180]  </TASK>

Next time please run a full fstests run, with CONFIG_BTRFS_ASSERT, 
CONFIG_BTRFS_DEBUG and CONFIG_BTRFS_EXPERIMENTAL enabled.

Now removed from for-next branch.

Thanks,
Qu

>   fs/btrfs/extent-io-tree.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
> index 626702244809..b5ba650cdb55 100644
> --- a/fs/btrfs/extent-io-tree.c
> +++ b/fs/btrfs/extent-io-tree.c
> @@ -663,6 +663,9 @@ int btrfs_clear_extent_bit_changeset(struct extent_io_tree *tree, u64 start, u64
>   		 */
>   		prealloc = alloc_extent_state(mask);
>   	}
> +	/* Preallocate the extent changeset ulist node before acquiring spinlock. */
> +	if (changeset)
> +		extent_changeset_prealloc(changeset, mask);
>   
>   	spin_lock(&tree->lock);
>   	if (cached_state) {