Re: [PATCH net-next v10 06/14] page_pool: convert to use netmem

Pavel Begunkov
2024-06-07T12:31:08Z

On 6/6/24 02:48, Steven Rostedt wrote:
> On Thu, 30 May 2024 20:16:05 +0000
> Mina Almasry <almasrymina@google.com> wrote:
>
>> @@ -42,51 +42,52 @@ TRACE_EVENT(page_pool_release,
>> TRACE_EVENT(page_pool_state_release,
>>
>> TP_PROTO(const struct page_pool *pool,
>> - const struct page *page, u32 release),
>> + netmem_ref netmem, u32 release),
>>
>> - TP_ARGS(pool, page, release),
>> + TP_ARGS(pool, netmem, release),
>>
>> TP_STRUCT__entry(
>> __field(const struct page_pool *, pool)
>> - __field(const struct page *, page)
>> + __field(netmem_ref, netmem)
>
> Why make this of type "netmem_ref" and not just "unsigned long"?
>
>> __field(u32, release)
>> __field(unsigned long, pfn)
>> ),
>>
>> TP_fast_assign(
>> __entry->pool = pool;
>> - __entry->page = page;
>> + __entry->netmem = netmem;
>
> You could have this be:
>
> __entry->netmem = (__force unsigned long)netmem;
>
>> __entry->release = release;
>> - __entry->pfn = page_to_pfn(page);
>> + __entry->pfn = netmem_to_pfn(netmem);
>> ),
>>
>> - TP_printk("page_pool=%p page=%p pfn=0x%lx release=%u",
>> - __entry->pool, __entry->page, __entry->pfn, __entry->release)
>> + TP_printk("page_pool=%p netmem=%lu pfn=0x%lx release=%u",
>> + __entry->pool, (__force unsigned long)__entry->netmem,
>
> And not have to expose the above text to user space (look at the format
> file it produces).
>
> It being of type "netmem_ref" in the ring buffer is useless.

netmem is a pointer with one bit serving as a flag, considering
mangling it might be better to %p it and perhaps also print its
type (page* vs iov) separately.

--
Pavel Begunkov