Messages in this thread |  | | From | Shay Agroskin <> | Subject | Re: [PATCH net-next v3 0/5] page_pool: recycle buffers | Date | Sun, 9 May 2021 08:11:35 +0300 |
| |
Jesper Dangaard Brouer <brouer@redhat.com> writes:
> On Fri, 7 May 2021 16:28:30 +0800 > Yunsheng Lin <linyunsheng@huawei.com> wrote: > >> On 2021/5/7 15:06, Ilias Apalodimas wrote: >> > On Fri, May 07, 2021 at 11:23:28AM +0800, Yunsheng Lin wrote: >> >> On 2021/5/6 20:58, Ilias Apalodimas wrote: >> >>>>>> >> >>>>> > ... >> > >> > >> > 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.
|  |