Messages in this thread | | | Date | Wed, 8 May 2024 12:30:07 +0100 | Subject | Re: [RFC PATCH net-next v8 02/14] net: page_pool: create hooks for custom page providers | From | Pavel Begunkov <> |
| |
On 5/8/24 00:32, Jason Gunthorpe wrote: > On Tue, May 07, 2024 at 08:35:37PM +0100, Pavel Begunkov wrote: >> On 5/7/24 18:56, Jason Gunthorpe wrote: >>> On Tue, May 07, 2024 at 06:25:52PM +0100, Pavel Begunkov wrote: >>>> On 5/7/24 17:48, Jason Gunthorpe wrote: >>>>> On Tue, May 07, 2024 at 09:42:05AM -0700, Mina Almasry wrote: >>>>> >>>>>> 1. Align with devmem TCP to use udmabuf for your io_uring memory. I >>>>>> think in the past you said it's a uapi you don't link but in the face >>>>>> of this pushback you may want to reconsider. >>>>> >>>>> dmabuf does not force a uapi, you can acquire your pages however you >>>>> want and wrap them up in a dmabuf. No uapi at all. >>>>> >>>>> The point is that dmabuf already provides ops that do basically what >>>>> is needed here. We don't need ops calling ops just because dmabuf's >>>>> ops are not understsood or not perfect. Fixup dmabuf. >>>> >>>> Those ops, for example, are used to efficiently return used buffers >>>> back to the kernel, which is uapi, I don't see how dmabuf can be >>>> fixed up to cover it. >>> >>> Sure, but that doesn't mean you can't use dma buf for the other parts >>> of the flow. The per-page lifetime is a different topic than the >>> refcounting and access of the entire bulk of memory. >> >> Ok, so if we're leaving uapi (and ops) and keep per page/sub-buffer as >> is, the rest is resolving uptr -> pages, and passing it to page pool in >> a convenient to page pool format (net_iov). > > I'm not going to pretend to know about page pool details, but dmabuf > is the way to get the bulk of pages into a pool within the net stack's > allocator and keep that bulk properly refcounted while.> > An object like dmabuf is needed for the general case because there are > not going to be per-page references or otherwise available.
They are already pinned, memory is owned by the provider, io_uring in this case, and it should not be freed circumventing io_uring, and at this stage calling release_pages() is not such a hassle, especially comparing to introducing an additional object.
My question is how having an intermediary dmabuf benefits the net stack or io_uring ? For now IMO it doesn't solve anything but adds extra complexity. Adding dmabuf for the sake of adding dmabuf is not a great choice.
> What you seem to want is to alter how the actual allocation flow works > from that bulk of memory and delay the free. It seems like a different For people who jumped here without looking what this patchset is about, that's the entire point of the io_uring zero copy approach as well as this set. Instead of using kernel private pages that you have no other option but to copy/mmap (and then free), it hands buffers to the user while using memory accessible/visible in some way by the user.
That "delay free" is taking a reference while user is reading data (slightly different for devmem tcp). And note, it's not a page/dmabuf reference, kernel can forcibly take it back and release pages.
> topic to me, and honestly hacking into the allocator free function > seems a bit weird..
Do you also think that DMA_BUF_IOCTL_SYNC is a weird hack, because it "delays free" by pinning the dmabuf object and letting the user read memory instead of copying it? I can find many examples
-- Pavel Begunkov
| |