[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAKgT0Ueg1u2S5LJuo0Ecs9dAPPDujtJ0GLcm8BTsfDx9LpJZVg@mail.gmail.com>
Date: Sat, 29 Jun 2024 10:37:43 -0700
From: Alexander Duyck <alexander.duyck@...il.com>
To: Yunsheng Lin <linyunsheng@...wei.com>
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org
Subject: Re: [PATCH net-next v9 10/13] mm: page_frag: introduce
prepare/probe/commit API
On Sat, Jun 29, 2024 at 4:15 AM Yunsheng Lin <linyunsheng@...wei.com> wrote:
>
> On 2024/6/29 6:35, Alexander H Duyck wrote:
> > On Tue, 2024-06-25 at 21:52 +0800, Yunsheng Lin wrote:
> >> There are many use cases that need minimum memory in order
> >> for forward progress, but more performant if more memory is
> >> available or need to probe the cache info to use any memory
> >> available for frag caoleasing reason.
> >>
> >> Currently skb_page_frag_refill() API is used to solve the
> >> above use cases, but caller needs to know about the internal
> >> detail and access the data field of 'struct page_frag' to
> >> meet the requirement of the above use cases and its
> >> implementation is similar to the one in mm subsystem.
> >>
> >> To unify those two page_frag implementations, introduce a
> >> prepare API to ensure minimum memory is satisfied and return
> >> how much the actual memory is available to the caller and a
> >> probe API to report the current available memory to caller
> >> without doing cache refilling. The caller needs to either call
> >> the commit API to report how much memory it actually uses, or
> >> not do so if deciding to not use any memory.
> >>
> >> As next patch is about to replace 'struct page_frag' with
> >> 'struct page_frag_cache' in linux/sched.h, which is included
> >> by the asm-offsets.s, using the virt_to_page() in the inline
> >> helper of page_frag_cache.h cause a "'vmemmap' undeclared"
> >> compiling error for asm-offsets.s, use a macro for probe API
> >> to avoid that compiling error.
> >>
> >> CC: Alexander Duyck <alexander.duyck@...il.com>
> >> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>
> >> ---
> >> include/linux/page_frag_cache.h | 82 +++++++++++++++++++++++
> >> mm/page_frag_cache.c | 114 ++++++++++++++++++++++++++++++++
> >> 2 files changed, 196 insertions(+)
> >>
> >> diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
> >> index b33904d4494f..e95d44a36ec9 100644
> >> --- a/include/linux/page_frag_cache.h
> >> +++ b/include/linux/page_frag_cache.h
> >> @@ -4,6 +4,7 @@
> >> #define _LINUX_PAGE_FRAG_CACHE_H
> >>
> >> #include <linux/gfp_types.h>
> >> +#include <linux/mmdebug.h>
> >>
> >> #define PAGE_FRAG_CACHE_MAX_SIZE __ALIGN_MASK(32768, ~PAGE_MASK)
> >> #define PAGE_FRAG_CACHE_MAX_ORDER get_order(PAGE_FRAG_CACHE_MAX_SIZE)
> >> @@ -87,6 +88,9 @@ static inline unsigned int page_frag_cache_page_size(struct encoded_va *encoded_
> >>
> >> void page_frag_cache_drain(struct page_frag_cache *nc);
> >> void __page_frag_cache_drain(struct page *page, unsigned int count);
> >> +struct page *page_frag_alloc_pg(struct page_frag_cache *nc,
> >> + unsigned int *offset, unsigned int fragsz,
> >> + gfp_t gfp);
> >> void *__page_frag_alloc_va_align(struct page_frag_cache *nc,
> >> unsigned int fragsz, gfp_t gfp_mask,
> >> unsigned int align_mask);
> >> @@ -99,12 +103,90 @@ static inline void *page_frag_alloc_va_align(struct page_frag_cache *nc,
> >> return __page_frag_alloc_va_align(nc, fragsz, gfp_mask, -align);
> >> }
> >>
> >> +static inline unsigned int page_frag_cache_page_offset(const struct page_frag_cache *nc)
> >> +{
> >> + return page_frag_cache_page_size(nc->encoded_va) - nc->remaining;
> >> +}
> >> +
> >> static inline void *page_frag_alloc_va(struct page_frag_cache *nc,
> >> unsigned int fragsz, gfp_t gfp_mask)
> >> {
> >> return __page_frag_alloc_va_align(nc, fragsz, gfp_mask, ~0u);
> >> }
> >>
> >> +void *page_frag_alloc_va_prepare(struct page_frag_cache *nc, unsigned int *fragsz,
> >> + gfp_t gfp);
> >> +
> >> +static inline void *page_frag_alloc_va_prepare_align(struct page_frag_cache *nc,
> >> + unsigned int *fragsz,
> >> + gfp_t gfp,
> >> + unsigned int align)
> >> +{
> >> + WARN_ON_ONCE(!is_power_of_2(align) || align > PAGE_SIZE);
> >> + nc->remaining = nc->remaining & -align;
> >> + return page_frag_alloc_va_prepare(nc, fragsz, gfp);
> >> +}
> >> +
> >> +struct page *page_frag_alloc_pg_prepare(struct page_frag_cache *nc,
> >> + unsigned int *offset,
> >> + unsigned int *fragsz, gfp_t gfp);
> >> +
> >> +struct page *page_frag_alloc_prepare(struct page_frag_cache *nc,
> >> + unsigned int *offset,
> >> + unsigned int *fragsz,
> >> + void **va, gfp_t gfp);
> >> +
> >> +static inline struct encoded_va *__page_frag_alloc_probe(struct page_frag_cache *nc,
> >> + unsigned int *offset,
> >> + unsigned int *fragsz,
> >> + void **va)
> >> +{
> >> + struct encoded_va *encoded_va;
> >> +
> >> + *fragsz = nc->remaining;
> >> + encoded_va = nc->encoded_va;
> >> + *offset = page_frag_cache_page_size(encoded_va) - *fragsz;
> >> + *va = encoded_page_address(encoded_va) + *offset;
> >> +
> >> + return encoded_va;
> >> +}
> >> +
> >> +#define page_frag_alloc_probe(nc, offset, fragsz, va) \
> >> +({ \
> >> + struct page *__page = NULL; \
> >> + \
> >> + VM_BUG_ON(!*(fragsz)); \
> >> + if (likely((nc)->remaining >= *(fragsz))) \
> >> + __page = virt_to_page(__page_frag_alloc_probe(nc, \
> >> + offset, \
> >> + fragsz, \
> >> + va)); \
> >> + \
> >> + __page; \
> >> +})
> >> +
> >
> > Why is this a macro instead of just being an inline? Are you trying to
> > avoid having to include a header due to the virt_to_page?
>
> Yes, you are right.
> I tried including different headers for virt_to_page(), and it did not
> work for arch/x86/kernel/asm-offsets.s, which has included linux/sched.h,
> and linux/sched.h need 'struct page_frag_cache' for 'struct task_struct'
> after this patchset, including page_frag_cache.h for sched.h causes the
> below compiler error:
>
> CC arch/x86/kernel/asm-offsets.s
> In file included from ./arch/x86/include/asm/page.h:89,
> from ./arch/x86/include/asm/thread_info.h:12,
> from ./include/linux/thread_info.h:60,
> from ./include/linux/spinlock.h:60,
> from ./include/linux/swait.h:7,
> from ./include/linux/completion.h:12,
> from ./include/linux/crypto.h:15,
> from arch/x86/kernel/asm-offsets.c:9:
> ./include/linux/page_frag_cache.h: In function ‘page_frag_alloc_align’:
> ./include/asm-generic/memory_model.h:37:34: error: ‘vmemmap’ undeclared (first use in this function); did you mean ‘mem_map’?
> 37 | #define __pfn_to_page(pfn) (vmemmap + (pfn))
> | ^~~~~~~
> ./include/asm-generic/memory_model.h:65:21: note: in expansion of macro ‘__pfn_to_page’
> 65 | #define pfn_to_page __pfn_to_page
> | ^~~~~~~~~~~~~
> ./arch/x86/include/asm/page.h:68:33: note: in expansion of macro ‘pfn_to_page’
> 68 | #define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> | ^~~~~~~~~~~
> ./include/linux/page_frag_cache.h:151:16: note: in expansion of macro ‘virt_to_page’
> 151 | return virt_to_page(va);
> | ^~~~~~~~~~~~
> ./include/asm-generic/memory_model.h:37:34: note: each undeclared identifier is reported only once for each function it appears in
> 37 | #define __pfn_to_page(pfn) (vmemmap + (pfn))
> | ^~~~~~~
> ./include/asm-generic/memory_model.h:65:21: note: in expansion of macro ‘__pfn_to_page’
> 65 | #define pfn_to_page __pfn_to_page
> | ^~~~~~~~~~~~~
> ./arch/x86/include/asm/page.h:68:33: note: in expansion of macro ‘pfn_to_page’
> 68 | #define virt_to_page(kaddr) pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
> | ^~~~~~~~~~~
> ./include/linux/page_frag_cache.h:151:16: note: in expansion of macro ‘virt_to_page’
> 151 | return virt_to_page(va);
>
>
I am pretty sure you just need to add:
#include <asm/page.h>
Powered by blists - more mailing lists