From: Jakub Kicinski <kuba@xxxxxxxxxx> 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@xxxxxxxxx/ >> >> 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