Re: [PATCH net-next v12 06/13] page_pool: devmem support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/21/24 19:48, Mina Almasry wrote:
On Mon, Jun 17, 2024 at 7:17 AM Pavel Begunkov <asml.silence@xxxxxxxxx> wrote:
...
   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


+
       return page_to_pfn(netmem_to_page(netmem));
   }

...
   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



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux