| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAKgT0UdHrEzXwceS-5m1Hc1dV9r_XiPjSSc=_vWCUu0C5pfE4w@mail.gmail.com>
Date: Sun, 21 Jul 2024 10:58:21 -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,
David Howells <dhowells@...hat.com>, Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org
Subject: Re: [RFC v11 02/14] mm: move the page fragment allocator from
page_alloc into its own file
On Fri, Jul 19, 2024 at 2:37 AM Yunsheng Lin <linyunsheng@...wei.com> 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@redhat.com/
> 2. https://lore.kernel.org/all/15623dac-9358-4597-b3ee-3694a5956920@gmail.com/
> 3. https://lore.kernel.org/all/CAKgT0UdH1yD=LSCXFJ=YM_aiA4OomD-2wXykO42bizaWMt_HOA@mail.gmail.com/
> CC: David Howells <dhowells@...hat.com>
> CC: Alexander Duyck <alexander.duyck@...il.com>
> Signed-off-by: Yunsheng Lin <linyunsheng@...wei.com>
> ---
> 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@...wei.com
> */
>
> -#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.
Powered by blists - more mailing lists