Messages in this thread | | | Date | Mon, 13 Nov 2023 18:05:54 -0500 | From | Jakub Kicinski <> | Subject | Re: [PATCH RFC 3/8] memory-provider: dmabuf devmem memory provider |
| |
On Mon, 13 Nov 2023 05:42:16 -0800 Mina Almasry wrote: > You're doing exactly what I think you're doing, and what was nacked in RFC v1. > > You've converted 'struct page_pool_iov' to essentially become a > duplicate of 'struct page'. Then, you're casting page_pool_iov* into > struct page* in mp_dmabuf_devmem_alloc_pages(), then, you're calling > mm APIs like page_ref_*() on the page_pool_iov* because you've fooled > the mm stack into thinking dma-buf memory is a struct page. > > RFC v1 was almost exactly the same, except instead of creating a > duplicate definition of struct page, it just allocated 'struct page' > instead of allocating another struct that is identical to struct page > and casting it into struct page. > > I don't think what you're doing here reverses the nacks I got in RFC > v1. You also did not CC any dma-buf or mm people on this proposal that > would bring up these concerns again.
Right, but the mirror struct has some appeal to a non-mm person like myself. The problem IIUC is that this patch is the wrong way around, we should be converting everyone who can deal with non-host mem to struct page_pool_iov. Using page_address() on ppiov which hns3 seems to do in this series does not compute for me.
Then we can turn the existing non-iov helpers to be a thin wrapper with just a cast from struct page to struct page_pool_iov, and a call of the iov helper. Again - never cast the other way around.
Also I think this conversion can be done completely separately from the mem provider changes. Just add struct page_pool_iov and start using it.
Does that make more sense?
| |