Re: [PATCH v4 04/18] btrfs: make attach_extent_buffer_page() to handle subpage case

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2021/1/20 上午6:35, David Sterba wrote:
> On Tue, Jan 19, 2021 at 04:54:28PM -0500, Josef Bacik wrote:
>> On 1/16/21 2:15 AM, Qu Wenruo wrote:
>>> +/* For rare cases where we need to pre-allocate a btrfs_subpage structure */
>>> +static inline int btrfs_alloc_subpage(struct btrfs_fs_info *fs_info,
>>> +				      struct btrfs_subpage **ret)
>>> +{
>>> +	if (fs_info->sectorsize == PAGE_SIZE)
>>> +		return 0;
>>> +
>>> +	*ret = kzalloc(sizeof(struct btrfs_subpage), GFP_NOFS);
>>> +	if (!*ret)
>>> +		return -ENOMEM;
>>> +	return 0;
>>> +}
>>
>> We're allocating these for every metadata page, that deserves a dedicated
>> kmem_cache.  Thanks,
>
> I'm not opposed to that idea but for the first implementation I'm ok
> with using the default slabs. As the subpage support depends on the
> filesystem, creating the cache unconditionally would waste resources and
> creating it on demand would need some care. Either way I'd rather see it
> in a separate patch.
>
Well, too late for me to see this comment...

As I have already converted to kmem cache.

But the good news is, the latest version has extra refactor on memory
allocation/freeing, now we just need to change two lines to change how
we allocate memory for subpage.
(Although still need to remove the cache allocation code).

Will convert it back to default slab, but will also keep the refactor
there to make it easier for later convert to kmem_cache.

So don't be too surprised to see function like in next version.

   btrfs_free_subpage(struct btrfs_subpage *subpage)
   {
	kfree(subpage);
   }

Thanks,
Qu