[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <baa2238a-6af9-ae19-0383-9e279c0a7fcf@kernel.org>
Date: Fri, 10 May 2024 10:38:12 -0700 (PDT)
From: Mat Martineau <martineau@...nel.org>
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,
Alexander Duyck <alexander.duyck@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org
Subject: Re: [PATCH net-next v3 10/13] mm: page_frag: introduce prepare/probe/commit
API
On Wed, 8 May 2024, Yunsheng Lin wrote:
> There are many use cases that need minimum memory in order
> for forward progressing, 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 usecases, caller need 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 | 86 ++++++++++++++++++++++++
> mm/page_frag_cache.c | 113 ++++++++++++++++++++++++++++++++
> 2 files changed, 199 insertions(+)
>
> diff --git a/include/linux/page_frag_cache.h b/include/linux/page_frag_cache.h
> index 88e91ee57b91..30893638155b 100644
> --- a/include/linux/page_frag_cache.h
> +++ b/include/linux/page_frag_cache.h
> @@ -71,6 +71,21 @@ static inline bool page_frag_cache_is_pfmemalloc(struct page_frag_cache *nc)
> return encoded_page_pfmemalloc(nc->encoded_va);
> }
>
> +static inline unsigned int page_frag_cache_page_size(struct encoded_va *encoded_va)
> +{
> +#if (PAGE_SIZE < PAGE_FRAG_CACHE_MAX_SIZE)
> + return PAGE_SIZE << encoded_page_order(encoded_va);
> +#else
> + return PAGE_SIZE;
> +#endif
> +}
> +
> +static inline unsigned int __page_frag_cache_page_offset(struct encoded_va *encoded_va,
> + unsigned int remaining)
> +{
> + return page_frag_cache_page_size(encoded_va) - remaining;
> +}
> +
> 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_va_align(struct page_frag_cache *nc,
> @@ -85,12 +100,83 @@ 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_offset(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_offset(encoded_va, *fragsz);
> + *va = encoded_page_address(encoded_va) + *offset;
> +
> + return encoded_va;
> +}
> +
> +#define page_frag_alloc_probe(nc, offset, fragsz, va) \
> +({ \
> + struct encoded_va *__encoded_va; \
> + struct page *__page = NULL; \
> + \
Hi Yunsheng -
I made this suggestion for patch 13 (documentation), but want to clarify
my request here:
> + if (likely((nc)->remaining)) \
I think it would be more useful to change this line to
if ((nc)->remaining >= *fragsz)
That way the caller can use this function to "probe" for a specific amount
of available space, rather than "nonzero" space. If the caller wants to
check for available space, they can set *fragsz = 1.
In other words, I think the functionality you described in the
documentation is better and the code should be changed to match!
- Mat
> + __page = virt_to_page(__page_frag_alloc_probe(nc, \
> + offset, \
> + fragsz, \
> + va)); \
> + \
> + __page; \
> +})
> +
> +static inline void page_frag_alloc_commit(struct page_frag_cache *nc,
> + unsigned int fragsz)
> +{
> + VM_BUG_ON(fragsz > nc->remaining || !nc->pagecnt_bias);
> + nc->pagecnt_bias--;
> + nc->remaining -= fragsz;
> +}
> +
> +static inline void page_frag_alloc_commit_noref(struct page_frag_cache *nc,
> + unsigned int fragsz)
> +{
> + VM_BUG_ON(fragsz > nc->remaining);
> + nc->remaining -= fragsz;
> +}
> +
> void page_frag_free_va(void *addr);
>
> #endif
Powered by blists - more mailing lists