On Tue, Jan 26, 2021 at 03:29:17PM +0800, Qu Wenruo wrote:
> 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);
> }
I hoped to save you time converting it to the kmem slabs so no need to
revert it back to kmalloc, keep what you have. Switching with the helper
would be easier should we need to reconsider it for some reason.
