lkml.org 
[lkml]   [2021]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH net-next v3 0/5] page_pool: recycle buffers
Date

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.

\
 
 \ /
  Last update: 2021-05-09 07:13    [W:0.199 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site