Re: [RFC v11 02/14] mm: move the page fragment allocator from page_alloc into its own file

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/22/2024 1:58 AM, Alexander Duyck wrote:
On Fri, Jul 19, 2024 at 2:37 AM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote:

...

--- /dev/null
+++ b/include/linux/page_frag_cache.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_PAGE_FRAG_CACHE_H
+#define _LINUX_PAGE_FRAG_CACHE_H
+
+#include <linux/log2.h>
+#include <linux/types.h>
+#include <linux/mm_types_task.h>

You don't need to include mm_types_task.h here. You can just use
declare "struct page_frag_cache;" as we did before in gfp.h.
Technically this should be included in mm_types.h so any callers
making use of these functions would need to make sure to include that
like we did for gfp.h before anyway.

The probe API is added as an inline helper in patch 11 according to
discussion in [1], so the definition of "struct page_frag_cache" is
needed, so I am not sure what is the point of using
"struct page_frag_cache;" here and then remove it and include
mm_types_task.h in patch 11.

1. https://lore.kernel.org/all/cb541985-a06d-7a71-9e6d-38827ccdf875@xxxxxxxxxx/


+#include <asm/page.h>
+

Not sure why this is included here either. From what I can tell there
isn't anything here using the contents of page.h. I suspect you should
only need it for the get_order call which would be used in other
files.

It seems unnecessay, will remove that.


+void page_frag_cache_drain(struct page_frag_cache *nc);
+void __page_frag_cache_drain(struct page *page, unsigned int count);
+void *__page_frag_alloc_align(struct page_frag_cache *nc, unsigned int fragsz,
+                             gfp_t gfp_mask, unsigned int align_mask);
+
+static inline void *page_frag_alloc_align(struct page_frag_cache *nc,
+                                         unsigned int fragsz, gfp_t gfp_mask,
+                                         unsigned int align)
+{
+       WARN_ON_ONCE(!is_power_of_2(align));
+       return __page_frag_alloc_align(nc, fragsz, gfp_mask, -align);
+}
+
+static inline void *page_frag_alloc(struct page_frag_cache *nc,
+                                   unsigned int fragsz, gfp_t gfp_mask)
+{
+       return __page_frag_alloc_align(nc, fragsz, gfp_mask, ~0u);
+}
+
+void page_frag_free(void *addr);
+
+#endif

...

diff --git a/mm/page_frag_test.c b/mm/page_frag_test.c
index cf2691f60b67..b7a5affb92f2 100644
--- a/mm/page_frag_test.c
+++ b/mm/page_frag_test.c
@@ -6,7 +6,6 @@
   * Copyright: linyunsheng@xxxxxxxxxx
   */

-#include <linux/mm.h>
  #include <linux/module.h>
  #include <linux/slab.h>
  #include <linux/vmalloc.h>
@@ -16,6 +15,7 @@
  #include <linux/log2.h>
  #include <linux/completion.h>
  #include <linux/kthread.h>
+#include <linux/page_frag_cache.h>

  #define OBJPOOL_NR_OBJECT_MAX  BIT(24)

Rather than making users have to include page_frag_cache.h I think it
would be better for us to just maintain the code as being accessible
from mm.h. So it might be better to just add page_frag_cache.h to the
includes there.

It would be better to list out why it is better that way as I am failing
to see it that way yet as I think it is better to use the explicit
header file instead the implicit header file.








[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux