Messages in this thread | ![/](/images/icornerl.gif) | | From | Mina Almasry <> | Date | Tue, 5 Mar 2024 18:42:35 -0800 | Subject | Re: [RFC PATCH net-next v6 09/15] memory-provider: dmabuf devmem memory provider |
| |
On Tue, Mar 5, 2024 at 6:28 PM David Wei <dw@davidwei.uk> wrote: > > On 2024-03-04 18:01, Mina Almasry wrote: > > + if (pool->p.queue) > > + binding = READ_ONCE(pool->p.queue->binding); > > + > > + if (binding) { > > + pool->mp_ops = &dmabuf_devmem_ops; > > + pool->mp_priv = binding; > > + } > > This is specific to TCP devmem. For ZC Rx we will need something more > generic to let us pass our own memory provider backend down to the page > pool. > > What about storing ops and priv void ptr in struct netdev_rx_queue > instead? Then we can both use it.
Yes, this is dmabuf specific, I was thinking you'd define your own member of netdev_rx_queue, and then add something like this to page_pool_init:
+ if (pool->p.queue) + io_uring_metadata = READ_ONCE(pool->p.queue->io_uring_metadata); + + /* We don't support rx-queues that are configured for both io_uring & dmabuf binding */ + BUG_ON(io_uring_metadata && binding); + + if (io_uring_metadata) { + pool->mp_ops = &io_uring_ops; + pool->mp_priv = io_uring_metadata; + }
I.e., we share the pool->mp_ops and the pool->mp_priv but we don't really need to share the same netdev_rx_queue member. For me it's a dma-buf specific data structure (netdev_dmabuf_binding) and for you it's something else.
page_pool_init() probably needs to validate that the queue is configured for dma-buf or io_uring but not both. If it's configured for both then the user is doing something funky we shouldn't support.
Perhaps I can make the intention clearer by renaming 'binding' to something more specific to dma-buf like queue->dmabuf_binding, to make it clear that this is the dma-buf binding and not some other binding like io_uring?
-- Thanks, Mina
| ![\](/images/icornerr.gif) |