[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87bk9416vq.fsf@toke.dk>
Date: Mon, 29 Jan 2024 16:44:41 +0100
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Lorenzo Bianconi <lorenzo.bianconi@...hat.com>
Cc: Lorenzo Bianconi <lorenzo@...nel.org>, netdev@...r.kernel.org,
davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com,
pabeni@...hat.com, bpf@...r.kernel.org, willemdebruijn.kernel@...il.com,
jasowang@...hat.com, sdf@...gle.com, hawk@...nel.org,
ilias.apalodimas@...aro.org
Subject: Re: [PATCH v6 net-next 1/5] net: add generic per-cpu page_pool
allocator
Lorenzo Bianconi <lorenzo.bianconi@...hat.com> writes:
>> Lorenzo Bianconi <lorenzo@...nel.org> writes:
>>
>> > Introduce generic percpu page_pools allocator.
>> > Moreover add page_pool_create_percpu() and cpuid filed in page_pool struct
>> > in order to recycle the page in the page_pool "hot" cache if
>> > napi_pp_put_page() is running on the same cpu.
>> > This is a preliminary patch to add xdp multi-buff support for xdp running
>> > in generic mode.
>> >
>> > Signed-off-by: Lorenzo Bianconi <lorenzo@...nel.org>
>> > ---
>> > include/net/page_pool/types.h | 3 +++
>> > net/core/dev.c | 40 +++++++++++++++++++++++++++++++++++
>> > net/core/page_pool.c | 23 ++++++++++++++++----
>> > net/core/skbuff.c | 5 +++--
>> > 4 files changed, 65 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
>> > index 76481c465375..3828396ae60c 100644
>> > --- a/include/net/page_pool/types.h
>> > +++ b/include/net/page_pool/types.h
>> > @@ -128,6 +128,7 @@ struct page_pool_stats {
>> > struct page_pool {
>> > struct page_pool_params_fast p;
>> >
>> > + int cpuid;
>> > bool has_init_callback;
>> >
>> > long frag_users;
>> > @@ -203,6 +204,8 @@ struct page *page_pool_alloc_pages(struct page_pool *pool, gfp_t gfp);
>> > struct page *page_pool_alloc_frag(struct page_pool *pool, unsigned int *offset,
>> > unsigned int size, gfp_t gfp);
>> > struct page_pool *page_pool_create(const struct page_pool_params *params);
>> > +struct page_pool *page_pool_create_percpu(const struct page_pool_params *params,
>> > + int cpuid);
>> >
>> > struct xdp_mem_info;
>> >
>> > diff --git a/net/core/dev.c b/net/core/dev.c
>> > index cb2dab0feee0..bf9ec740b09a 100644
>> > --- a/net/core/dev.c
>> > +++ b/net/core/dev.c
>> > @@ -153,6 +153,8 @@
>> > #include <linux/prandom.h>
>> > #include <linux/once_lite.h>
>> > #include <net/netdev_rx_queue.h>
>> > +#include <net/page_pool/types.h>
>> > +#include <net/page_pool/helpers.h>
>> >
>> > #include "dev.h"
>> > #include "net-sysfs.h"
>> > @@ -442,6 +444,8 @@ static RAW_NOTIFIER_HEAD(netdev_chain);
>> > DEFINE_PER_CPU_ALIGNED(struct softnet_data, softnet_data);
>> > EXPORT_PER_CPU_SYMBOL(softnet_data);
>> >
>> > +DEFINE_PER_CPU_ALIGNED(struct page_pool *, page_pool);
>>
>> I think we should come up with a better name than just "page_pool" for
>> this global var. In the code below it looks like it's a local variable
>> that's being referenced. Maybe "global_page_pool" or "system_page_pool"
>> or something along those lines?
>
> ack, I will fix it. system_page_pool seems better, agree?
Yeah, agreed :)
-Toke
Powered by blists - more mailing lists