On Fri, Jul 19, 2024 at 2:37 AM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote: > > Inspired by [1], move the page fragment allocator from page_alloc > into its own c file and header file, as we are about to make more > change for it to replace another page_frag implementation in > sock.c > > As this patchset is going to replace 'struct page_frag' with > 'struct page_frag_cache' in sched.h, including page_frag_cache.h > in sched.h has a compiler error caused by interdependence between > mm_types.h and mm.h for asm-offsets.c, see [2]. So avoid the compiler > error by moving 'struct page_frag_cache' to mm_types_task.h as > suggested by Alexander, see [3]. > > 1. https://lore.kernel.org/all/20230411160902.4134381-3-dhowells@xxxxxxxxxx/ > 2. https://lore.kernel.org/all/15623dac-9358-4597-b3ee-3694a5956920@xxxxxxxxx/ > 3. https://lore.kernel.org/all/CAKgT0UdH1yD=LSCXFJ=YM_aiA4OomD-2wXykO42bizaWMt_HOA@xxxxxxxxxxxxxx/ > CC: David Howells <dhowells@xxxxxxxxxx> > CC: Alexander Duyck <alexander.duyck@xxxxxxxxx> > Signed-off-by: Yunsheng Lin <linyunsheng@xxxxxxxxxx> > --- > include/linux/gfp.h | 22 ----- > include/linux/mm_types.h | 18 ---- > include/linux/mm_types_task.h | 18 ++++ > include/linux/page_frag_cache.h | 32 +++++++ > include/linux/skbuff.h | 1 + > mm/Makefile | 1 + > mm/page_alloc.c | 136 ------------------------------ > mm/page_frag_cache.c | 145 ++++++++++++++++++++++++++++++++ > mm/page_frag_test.c | 2 +- > 9 files changed, 198 insertions(+), 177 deletions(-) > create mode 100644 include/linux/page_frag_cache.h > create mode 100644 mm/page_frag_cache.c > ... > diff --git a/include/linux/mm_types_task.h b/include/linux/mm_types_task.h > index a2f6179b672b..cdc1e3696439 100644 > --- a/include/linux/mm_types_task.h > +++ b/include/linux/mm_types_task.h > @@ -8,6 +8,7 @@ > * (These are defined separately to decouple sched.h from mm_types.h as much as possible.) > */ > > +#include <linux/align.h> > #include <linux/types.h> > > #include <asm/page.h> > @@ -46,6 +47,23 @@ struct page_frag { > #endif > }; > > +#define PAGE_FRAG_CACHE_MAX_SIZE __ALIGN_MASK(32768, ~PAGE_MASK) > +#define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE) > +struct page_frag_cache { > + void *va; > +#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE) > + __u16 offset; > + __u16 size; > +#else > + __u32 offset; > +#endif > + /* we maintain a pagecount bias, so that we dont dirty cache line > + * containing page->_refcount every time we allocate a fragment. > + */ > + unsigned int pagecnt_bias; > + bool pfmemalloc; > +}; > + > /* Track pages that require TLB flushes */ > struct tlbflush_unmap_batch { > #ifdef CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH > diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h > new file mode 100644 > index 000000000000..43afb1bbcac9 > --- /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. > +#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. > +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.
