| Message ID | 499bc85ca1d96ec1f7daff6b7df4350dc50e9256.1707931443.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: page_pool: make page_pool_create inline | expand |
On Wed, 14 Feb 2024 19:01:28 +0100 Lorenzo Bianconi wrote: > Make page_pool_create utility routine inline a remove exported symbol. But why? If you add the kdoc back the LoC saved will be 1. > include/net/page_pool/types.h | 6 +++++- > net/core/page_pool.c | 10 ---------- > 2 files changed, 5 insertions(+), 11 deletions(-) No strong opinion, but if you want to do it please put the helper in helpers.h Let's keep the static inlines clearly separated.
> On Wed, 14 Feb 2024 19:01:28 +0100 Lorenzo Bianconi wrote: > > Make page_pool_create utility routine inline a remove exported symbol. > > But why? If you add the kdoc back the LoC saved will be 1. I would remove the symbol exported since it is just a wrapper for page_pool_create_percpu() > > > include/net/page_pool/types.h | 6 +++++- > > net/core/page_pool.c | 10 ---------- > > 2 files changed, 5 insertions(+), 11 deletions(-) > > No strong opinion, but if you want to do it please put the helper > in helpers.h Let's keep the static inlines clearly separated. ack, fine to me. I put it there since we already have some inlines in page_pool/types.h. Regards, Lorenzo
Hi Lorenzo, On Wed, 14 Feb 2024 at 21:14, Lorenzo Bianconi <lorenzo@kernel.org> wrote: > > > On Wed, 14 Feb 2024 19:01:28 +0100 Lorenzo Bianconi wrote: > > > Make page_pool_create utility routine inline a remove exported symbol. > > > > But why? If you add the kdoc back the LoC saved will be 1. > > I would remove the symbol exported since it is just a wrapper for > page_pool_create_percpu() I don't mind either. But the explanation above must be part of the commit message. Thanks /Ilias > > > > > > include/net/page_pool/types.h | 6 +++++- > > > net/core/page_pool.c | 10 ---------- > > > 2 files changed, 5 insertions(+), 11 deletions(-) > > > > No strong opinion, but if you want to do it please put the helper > > in helpers.h Let's keep the static inlines clearly separated. > > ack, fine to me. I put it there since we already have some inlines in > page_pool/types.h. > > Regards, > Lorenzo
On 01/03/2024 11.27, Ilias Apalodimas wrote: > > On Wed, 14 Feb 2024 at 21:14, Lorenzo Bianconi <lorenzo@kernel.org> wrote: >> >>> On Wed, 14 Feb 2024 19:01:28 +0100 Lorenzo Bianconi wrote: >>>> Make page_pool_create utility routine inline a remove exported symbol. >>> >>> But why? If you add the kdoc back the LoC saved will be 1. >> >> I would remove the symbol exported since it is just a wrapper for >> page_pool_create_percpu() > > I don't mind either. But the explanation above must be part of the > commit message. I hope my benchmark module will still work... https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/lib/bench_page_pool_simple.c#L181 --Jesper
Hi Jesper, On Fri, 1 Mar 2024 at 12:33, Jesper Dangaard Brouer <hawk@kernel.org> wrote: > > > > On 01/03/2024 11.27, Ilias Apalodimas wrote: > > > > On Wed, 14 Feb 2024 at 21:14, Lorenzo Bianconi <lorenzo@kernel.org> wrote: > >> > >>> On Wed, 14 Feb 2024 19:01:28 +0100 Lorenzo Bianconi wrote: > >>>> Make page_pool_create utility routine inline a remove exported symbol. > >>> > >>> But why? If you add the kdoc back the LoC saved will be 1. > >> > >> I would remove the symbol exported since it is just a wrapper for > >> page_pool_create_percpu() > > > > I don't mind either. But the explanation above must be part of the > > commit message. > > I hope my benchmark module will still work... > > https://github.com/netoptimizer/prototype-kernel/blob/master/kernel/lib/bench_page_pool_simple.c#L181 Since we rely on this so much, perhaps we should make an effort to merge it, even in a version that only works for x86. I know it's fairly easy to compile and run locally, but having it as part of the CI is better -- more people will be able to look at the results quickly and determine if we have a performance degradation Cheers /Ilias > > > --Jesper
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h index 3828396ae60c..0057e584df9a 100644 --- a/include/net/page_pool/types.h +++ b/include/net/page_pool/types.h @@ -203,9 +203,13 @@ struct page_pool { 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); +static inline struct page_pool * +page_pool_create(const struct page_pool_params *params) +{ + return page_pool_create_percpu(params, -1); +} struct xdp_mem_info; diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 89c835fcf094..6e0753e6a95b 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -289,16 +289,6 @@ page_pool_create_percpu(const struct page_pool_params *params, int cpuid) } EXPORT_SYMBOL(page_pool_create_percpu); -/** - * page_pool_create() - create a page pool - * @params: parameters, see struct page_pool_params - */ -struct page_pool *page_pool_create(const struct page_pool_params *params) -{ - return page_pool_create_percpu(params, -1); -} -EXPORT_SYMBOL(page_pool_create); - static void page_pool_return_page(struct page_pool *pool, struct page *page); noinline
Make page_pool_create utility routine inline a remove exported symbol. Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- include/net/page_pool/types.h | 6 +++++- net/core/page_pool.c | 10 ---------- 2 files changed, 5 insertions(+), 11 deletions(-)