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

Reply via email to