On 6/21/24 19:48, Mina Almasry wrote:

On Mon, Jun 17, 2024 at 7:17 AM Pavel Begunkov [off-list ref] wrote:

...

quoted

quoted

   static inline unsigned long netmem_to_pfn(netmem_ref netmem)
   {
+     if (netmem_is_net_iov(netmem))
+             return 0;

IIRC 0 is a valid pfn. Not much of a concern since it's
used only for tracing, but might make sense to pass some
invalid pfn if there is one

AFAIU all non-negative pfns are technically valid pfns if the machine
is big enough.

I could have this function return long long instead of unsigned long
so I can return a negative number for errors, and then cast to
unsigned long when I figure out it's actually a pfn. Seemed like such
a hassle especially since the call site is just tracing that I figured
it's not that worth it.

Yeah, sounds like an overkill for tracing

quoted

quoted

+
       return page_to_pfn(netmem_to_page(netmem));
   }

...

quoted

   static inline netmem_ref netmem_compound_head(netmem_ref netmem)
   {
+     /* niov are never compounded */
+     if (netmem_is_net_iov(netmem))
+             return netmem;
+
       return page_to_netmem(compound_head(netmem_to_page(netmem)));
   }

+static inline void *netmem_address(netmem_ref netmem)

I don't think it's used anywhere, do I miss it?

Ah, It's used by the GVE devmem implementation:
https://github.com/mina/linux/commit/da89baa81873d457cbf7b49ee6b4f0d66855b205

I could leave it out of this patch, then add it with the follow up GVE
devmem implementation, but I figured almost for sure drivers are going
to need this eventually, and it's small, so just put it here.

Either way is fine by me, checking the function is not leftovers


-- 
Pavel Begunkov