On 6/6/24 02:48, Steven Rostedt wrote:
On Thu, 30 May 2024 20:16:05 +0000
Mina Almasry <almasrym...@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

Reply via email to