[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8639b838-8284-05a2-dbc3-7e4cb45f163a@intel.com>
Date: Tue, 11 Jul 2023 12:59:00 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Jakub Kicinski <kuba@...nel.org>,
Yunsheng Lin <yunshenglin0825@...il.com>
CC: Yunsheng Lin <linyunsheng@...wei.com>, <davem@...emloft.net>,
<pabeni@...hat.com>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
Lorenzo Bianconi <lorenzo@...nel.org>,
Alexander Duyck <alexander.duyck@...il.com>,
Liang Chen <liangchen.linux@...il.com>,
Saeed Mahameed <saeedm@...dia.com>,
"Leon Romanovsky" <leon@...nel.org>,
Eric Dumazet <edumazet@...gle.com>,
"Jesper Dangaard Brouer" <hawk@...nel.org>,
Ilias Apalodimas <ilias.apalodimas@...aro.org>,
<linux-rdma@...r.kernel.org>
Subject: Re: [PATCH v5 RFC 1/6] page_pool: frag API support for 32-bit arch
with 64-bit DMA
From: Jakub Kicinski <kuba@...nel.org>
Date: Mon, 10 Jul 2023 11:38:41 -0700
> On Sun, 9 Jul 2023 20:54:12 +0800 Yunsheng Lin wrote:
>>> And the include is still here, too, eh..
>>
>> In V4, it has:
>>
>> --- a/include/net/page_pool.h
>> +++ b/include/net/page_pool.h
>> @@ -33,6 +33,7 @@
>> #include <linux/mm.h> /* Needed by ptr_ring */
>> #include <linux/ptr_ring.h>
>> #include <linux/dma-direction.h>
>> +#include <linux/dma-mapping.h>
>>
>> As dma_get_cache_alignment() defined in dma-mapping.h is used
>> here, so we need to include dma-mapping.h.
>>
>> I though the agreement is that this patch only remove the
>> "#include <linux/dma-direction.h>" as we dma-mapping.h has included
>> dma-direction.h.
>>
>> And Alexander will work on excluding page_pool.h from skbuff.h
>> https://lore.kernel.org/all/09842498-b3ba-320d-be8d-348b85e8d525@intel.com/
>>
>> Did I miss something obvious here? Or there is better way to do it
>> than the method discussed in the above thread?
>
> We're adding a ton of static inline functions to what is a fairly core
> header for networking, that's what re-triggered by complaint:
>
> include/net/page_pool.h | 179 ++++++++++++++----
>
> Maybe we should revisit the idea of creating a new header file for
> inline helpers... Olek, WDYT?
I'm fine with that, although ain't really able to work on this myself
now :s (BTW I almost finished Netlink bigints, just some more libie/IAVF
crap).
It just needs to be carefully designed, because if we want move ALL the
inlines to a new header, we may end up including 2 PP's headers in each
file. That's why I'd prefer "core/driver" separation. Let's say skbuff.c
doesn't need page_pool_create(), page_pool_alloc(), and so on, while
drivers don't need some of its internal functions.
OTOH after my patch it's included in only around 20-30 files on
allmodconfig. That is literally nothing comparing to e.g. kernel.h
(w/includes) :D
Thanks,
Olek
Powered by blists - more mailing lists