On Thu, Dec 21, 2023 at 3:23 PM Shakeel Butt <shakeelb@xxxxxxxxxx> wrote: > > On Wed, Dec 20, 2023 at 01:45:01PM -0800, Mina Almasry wrote: > > Add the netmem_ref type, an abstraction for network memory. > > > > To add support for new memory types to the net stack, we must first > > abstract the current memory type. Currently parts of the net stack > > use struct page directly: > > > > - page_pool > > - drivers > > - skb_frag_t > > > > Originally the plan was to reuse struct page* for the new memory types, > > and to set the LSB on the page* to indicate it's not really a page. > > However, for compiler type checking we need to introduce a new type. > > > > netmem_ref is introduced to abstract the underlying memory type. Currently > > it's a no-op abstraction that is always a struct page underneath. In > > parallel there is an undergoing effort to add support for devmem to the > > net stack: > > > > https://lore.kernel.org/netdev/20231208005250.2910004-1-almasrymina@xxxxxxxxxx/ > > > > Signed-off-by: Mina Almasry <almasrymina@xxxxxxxxxx> > > > > --- > > > > v3: > > > > - Modify struct netmem from a union of struct page + new types to an opaque > > netmem_ref type. I went with: > > > > +typedef void *__bitwise netmem_ref; > > > > rather than this that Jakub recommended: > > > > +typedef unsigned long __bitwise netmem_ref; > > > > Because with the latter the compiler issues warnings to cast NULL to > > netmem_ref. I hope that's ok. > > > > Can you share what the warning was? You might just need __force > attribute. However you might need this __force a lot. I wonder if you > can just follow struct encoded_page example verbatim here. > The warning is like so: ./include/net/page_pool/helpers.h: In function ‘page_pool_alloc’: ./include/linux/stddef.h:8:14: warning: returning ‘void *’ from a function with return type ‘netmem_ref’ {aka ‘long unsigned int’} makes integer from pointer without a cast [-Wint-conversion] 8 | #define NULL ((void *)0) | ^ ./include/net/page_pool/helpers.h:132:24: note: in expansion of macro ‘NULL’ 132 | return NULL; | ^~~~ And happens in all the code where: netmem_ref func() { return NULL; } It's fixable by changing the return to `return (netmem_ref NULL);` or `return 0;`, but I feel like netmem_ref should be some type which allows a cast from NULL implicitly. Also as you (and patchwork) noticed, __bitwise should not be used with void*; it's only meant for integer types. Sorry I missed that in the docs and was not running make C=2. -- Thanks, Mina
