Re: [PATCH 1/5] btrfs: factor init_extent_buffer from __alloc_extent_buffer

On 6/25/26 11:14 PM, Boris Burkov wrote:
> On Thu, Jun 25, 2026 at 04:26:56PM +0200, Johannes Thumshirn wrote:
>> On 6/24/26 12:35 AM, Boris Burkov wrote:
>>> In preparation for preallocating extent_buffer data, factor eb
>>> initialization away from specifically allocating it. This allows us to
>>> allocate the eb, bfs, folios, etc. together in the main search_slot code
>>> paths, but still share initialization code with the dummy/test/clone
>>> allocation paths.
>>>
>>> Signed-off-by: Boris Burkov <[email protected]>
>>> ---
>>>    fs/btrfs/extent_io.c | 20 ++++++++++++--------
>>>    1 file changed, 12 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>>> index 0edd532174fa..fa4cc8bcd1af 100644
>>> --- a/fs/btrfs/extent_io.c
>>> +++ b/fs/btrfs/extent_io.c
>>> @@ -3054,12 +3054,9 @@ void btrfs_uninhibit_all_eb_writeback(struct btrfs_trans_handle *trans)
>>>    	xa_destroy(&trans->writeback_inhibited_ebs);
>>>    }
>>> -static struct extent_buffer *__alloc_extent_buffer(struct btrfs_fs_info *fs_info,
>>> -						   u64 start)
>>> +static void init_extent_buffer(struct btrfs_fs_info *fs_info,
>>> +			       struct extent_buffer *eb, u64 start)
>>>    {
>>> -	struct extent_buffer *eb = NULL;
>>> -
>>> -	eb = kmem_cache_zalloc(extent_buffer_cache, GFP_NOFS|__GFP_NOFAIL);
>>>    	eb->start = start;
>>>    	eb->len = fs_info->nodesize;
>>>    	eb->fs_info = fs_info;
>>> @@ -3072,7 +3069,15 @@ static struct extent_buffer *__alloc_extent_buffer(struct btrfs_fs_info *fs_info
>>>    	refcount_set(&eb->refs, 1);
>>>    	ASSERT(eb->len <= BTRFS_MAX_METADATA_BLOCKSIZE);
>>> +}
>>> +
>>> +static struct extent_buffer *__alloc_extent_buffer(struct btrfs_fs_info *fs_info,
>>> +						   u64 start)
>>> +{
>>> +	struct extent_buffer *eb;
>>> +	eb = kmem_cache_zalloc(extent_buffer_cache, GFP_NOFS | __GFP_NOFAIL);
>>> +	init_extent_buffer(fs_info, eb, start);
>>>    	return eb;
>>>    }
>>> @@ -3476,9 +3481,8 @@ struct extent_buffer *alloc_extent_buffer(struct btrfs_fs_info *fs_info,
>>>    	if (eb)
>>>    		return eb;
>>> -	eb = __alloc_extent_buffer(fs_info, start);
>>> -	if (!eb)
>>> -		return ERR_PTR(-ENOMEM);
>>> +	eb = kmem_cache_zalloc(extent_buffer_cache, GFP_NOFS | __GFP_NOFAIL);
>>> +	init_extent_buffer(fs_info, eb, start);
>>>    	/*
>>>    	 * The reloc trees are just snapshots, so we need them to appear to be
>> This looks wrong to me. Not the split out code, but how it is used in
>> alloc_extent_buffer(). Either you keep calling __alloc_extent_buffer() in
>> alloc_extent_buffer() OR don't call init_extent_buffer() in
>> __alloc_extent_buffer(). I see this is an intermediate step (and I haven't
>> looked at the rest of the series yet) but it looks wrong.
>>
> The idea is that you eventually (optionally) have the allocation happen
> elsewhere, so alloc_extent_buffer should not call __alloc_extent_buffer
> any more. That should be clearer in the second patch, I think.
>
> I left __alloc_extent_buffer for btrfs_clone_extent_buffer() and
> alloc_dummy_extent_buffer(). Would you prefer me to delete
> __alloc_extent_buffer and open code it in those two callers?


Yes looking at it again with the complete series applied it make more 
sense. Sorry for the noise.