Re: [PATCH v5 RFC 1/6] page_pool: frag API support for 32-bit arch with 64-bit DMA

[Yunsheng Lin]

On 2023/7/13 1:26, Jakub Kicinski wrote:
> On Wed, 12 Jul 2023 20:34:12 +0800 Yunsheng Lin wrote:
>>>> C sources can include $path/page_pool.h, headers should generally only
>>>> include $path/page_pool/types.h.  
>>
>> Does spliting the page_pool.h as above fix the problem about including
>> a ton of static inline functions from "linux/dma-mapping.h" in skbuff.c?
>>
>> As the $path/page_pool/helpers.h which uses dma_get_cache_alignment()
>> must include the "linux/dma-mapping.h" which has dma_get_cache_alignment()
>> defining as a static inline function.
>> and if skbuff.c include $path/page_pool.h or $path/page_pool/helpers.h,
>> doesn't we still have the same problem? Or do I misunderstand something
>> here?
> 
> I should have clarified that "types.h" should also include pure
> function declarations (and possibly static line wrappers like
> pure get/set functions which only need locally defined types).

So "types.h" is not supposed/allowed to include any header and
it can include any function declarations and static line wrappers
which do not depend on any other header? It means we need to forward
declaring a lot of 'struct' type for function declarations, right?
If it is the case, the "types.h" does not seems to match it's
naming when we can not really define most of the 'struct' in "types.h",
such as 'struct page_pool' need to include some header in order to
have definition of 'struct delayed_work'.
Similar issue for 'helpers.h', as it will include most of the
definition of 'struct', which are not really helpers, right?

> 
> The skbuff.h only needs to include $path/page_pool/types.h, right?

It seems doable, it need trying to prove it is indeed that case.

> 
> I know that Olek has a plan to remove the skbuff dependency completely
> but functionally / for any future dependencies - this should work?

I am not experienced and confident enough to say about this for now.