Jesper Dangaard Brouer [off-list ref] writes:

On Fri, 7 May 2021 16:28:30 +0800
Yunsheng Lin [off-list ref] wrote:

quoted

On 2021/5/7 15:06, Ilias Apalodimas wrote:

quoted

On Fri, May 07, 2021 at 11:23:28AM +0800, Yunsheng Lin wrote:  

quoted

On 2021/5/6 20:58, Ilias Apalodimas wrote:  

quoted

quoted

quoted

quoted

...

quoted

quoted

I think both choices are sane.  What I am trying to explain 
here, is
regardless of what we choose now, we can change it in the 
future without
affecting the API consumers at all.  What will change 
internally is the way we
lookup the page pool pointer we are trying to recycle.  

It seems the below API need changing?
+static inline void skb_mark_for_recycle(struct sk_buff *skb, 
struct page *page,
+					struct xdp_mem_info *mem)

I don't think we need to change this API, to support future 
memory
models.  Notice that xdp_mem_info have a 'type' member.

Hi,
Providing that we will (possibly as a future optimization) store 
the pointer to the page pool in struct page instead of strcut 
xdp_mem_info, passing
xdp_mem_info * instead of struct page_pool * would mean that for 
every packet we'll need to call
             xa = rhashtable_lookup(mem_id_ht, &mem->id, 
             mem_id_rht_params);
             xa->page_pool;

which might pressure the Dcache to fetch a pointer that might be 
present already in cache as part of driver's data-structures.

I tend to agree with Yunsheng that it makes more sense to adjust 
the API for the clear use-case now rather than using xdp_mem_info 
indirection. It seems to me like
the page signature provides the same information anyway and allows 
to support different memory types.

Shay

Naming in Computer Science is a hard problem ;-). Something that 
seems
to confuse a lot of people is the naming of the struct 
"xdp_mem_info".  
Maybe we should have named it "mem_info" instead or 
"net_mem_info", as
it doesn't indicate that the device is running XDP.

I see XDP as the RX-layer before the network stack, that helps 
drivers
to support different memory models, also for handling normal 
packets
that doesn't get process by XDP, and the drivers doesn't even 
need to
support XDP to use the "xdp_mem_info" type.