On 2023/11/14 23:41, Willem de Bruijn wrote: >> >> I am not sure dma-buf maintainer's concern is still there with this patchset. >> >> Whatever name you calling it for the struct, however you arrange each field >> in the struct, some metadata is always needed for dmabuf to intergrate into >> page pool. >> >> If the above is true, why not utilize the 'struct page' to have more unified >> handling? > > My understanding is that there is a general preference to simplify struct > page, and at the least not move in the other direction by overloading the > struct in new ways. As my understanding, the new struct is just mirroring the struct page pool is already using, see: https://elixir.free-electrons.com/linux/v6.7-rc1/source/include/linux/mm_types.h#L119 If there is simplifying to the struct page_pool is using, I think the new stuct the devmem memory provider is using can adjust accordingly. As a matter of fact, I think the way 'struct page' for devmem is decoupled from mm subsystem may provide a way to simplify or decoupled the already existing 'struct page' used in netstack from mm subsystem, before this patchset, it seems we have the below types of 'struct page': 1. page allocated in the netstack using page pool. 2. page allocated in the netstack using buddy allocator. 3. page allocated in other subsystem and passed to the netstack, such as zcopy or spliced page? If we can decouple 'struct page' for devmem from mm subsystem, we may be able to decouple the above 'struct page' from mm subsystem one by one. > > If using struct page for something that is not memory, there is ZONE_DEVICE. > But using that correctly is non-trivial: > > https://lore.kernel.org/all/ZKyZBbKEpmkFkpWV@xxxxxxxx/ > > Since all we need is a handle that does not leave the network stack, > a network specific struct like page_pool_iov entirely avoids this issue. Yes, I am agree about the network specific struct. I am wondering if we can make the struct more generic if we want to intergrate it into page_pool and use it in net stack. > RFC v3 seems like a good simplification over RFC v1 in that regard to me. > I was also pleasantly surprised how minimal the change to the users of > skb_frag_t actually proved to be. Yes, I am agreed about that too. Maybe we can make it simpler by using a more abstract struct as page_pool, and utilize some features of page_pool too. For example, from the page_pool doc, page_pool have fast cache and ptr-ring cache as below, but if napi_frag_unref() call page_pool_page_put_many() and return the dmabuf chunk directly to gen_pool in the memory provider, then it seems we are bypassing the below caches in the page_pool. +------------------+ | Driver | +------------------+ ^ | | | v +--------------------------------------------+ | request memory | +--------------------------------------------+ ^ ^ | | | Pool empty | Pool has entries | | v v +-----------------------+ +------------------------+ | alloc (and map) pages | | get page from cache | +-----------------------+ +------------------------+ ^ ^ | | | cache available | No entries, refill | | from ptr-ring | | v v +-----------------+ +------------------+ | Fast cache | | ptr-ring cache | +-----------------+ +------------------+ > > . >
