Re: [PATCH net-next v13 07/14] mm: page_frag: reuse existing space for 'size' and 'pfmemalloc'

[Alexander Duyck <alexander.duyck@gmail.com>]

On Wed, Aug 14, 2024 at 8:10 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>
> On 2024/8/15 0:13, Alexander H Duyck wrote:
> > On Thu, 2024-08-08 at 20:37 +0800, Yunsheng Lin wrote:
> >> Currently there is one 'struct page_frag' for every 'struct
> >> sock' and 'struct task_struct', we are about to replace the
> >> 'struct page_frag' with 'struct page_frag_cache' for them.
> >> Before begin the replacing, we need to ensure the size of
> >> 'struct page_frag_cache' is not bigger than the size of
> >> 'struct page_frag', as there may be tens of thousands of
> >> 'struct sock' and 'struct task_struct' instances in the
> >> system.
> >>
> >> By or'ing the page order & pfmemalloc with lower bits of
> >> 'va' instead of using 'u16' or 'u32' for page size and 'u8'
> >> for pfmemalloc, we are able to avoid 3 or 5 bytes space waste.
> >> And page address & pfmemalloc & order is unchanged for the
> >> same page in the same 'page_frag_cache' instance, it makes
> >> sense to fit them together.
> >>
> >> After this patch, the size of 'struct page_frag_cache' should be
> >> the same as the size of 'struct page_frag'.
> >>
> >> CC: Alexander Duyck <alexander.duyck@gmail.com>
> >> Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> >> ---
> >>  include/linux/mm_types_task.h   | 16 +++++-----
> >>  include/linux/page_frag_cache.h | 52 +++++++++++++++++++++++++++++++--
> >>  mm/page_frag_cache.c            | 49 +++++++++++++++++--------------
> >>  3 files changed, 85 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/include/linux/mm_types_task.h b/include/linux/mm_types_task.h
> >> index b1c54b2b9308..f2610112a642 100644
> >> --- a/include/linux/mm_types_task.h
> >> +++ b/include/linux/mm_types_task.h
> >> @@ -50,18 +50,18 @@ struct page_frag {
> >>  #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)
> >> +    /* encoded_va consists of the virtual address, pfmemalloc bit and order
> >> +     * of a page.
> >> +     */
> >> +    unsigned long encoded_va;
> >> +
> >
> > Rather than calling this an "encoded_va" we might want to call this an
> > "encoded_page" as that would be closer to what we are actually working
> > with. We are just using the virtual address as the page pointer instead
> > of the page struct itself since we need quicker access to the virtual
> > address than we do the page struct.
>
> Calling it "encoded_page" seems confusing enough when calling virt_to_page()
> with "encoded_page" when virt_to_page() is expecting a 'va', no?

It makes about as much sense as calling it an "encoded_va". What you
have is essentially a packed page struct that contains the virtual
address, pfmemalloc flag, and order. So if you want you could call it
"packed_page" too I suppose. Basically this isn't a valid virtual
address it is a page pointer with some extra metadata packed in.