On 6/3/24 15:17, Mina Almasry wrote:
On Fri, May 31, 2024 at 10:35 PM Christoph Hellwig hch@infradead.org wrote:
On Thu, May 30, 2024 at 08:16:01PM +0000, Mina Almasry wrote:
I'm unsure if the discussion has been resolved yet. Sending the series anyway to get reviews/feedback on the (unrelated) rest of the series.
As far as I'm concerned it is not. I've not seen any convincing argument for more than page/folio allocator including larger order / huge page and dmabuf.
Thanks Christoph, this particular patch series adds dmabuf, so I assume no objection there. I assume the objection is that you want the generic, extensible hooks removed.
To be honest, I don't think the hooks are an integral part of the design, and at this point I think we've argued for them enough. I think we can easily achieve the same thing with just raw if statements in a couple of places. We can always add the hooks if and only if we actually justify many memory providers.
Any objections to me removing the hooks and directing to memory allocations via simple if statements? Something like (very rough draft, doesn't compile):
The question for Christoph is what exactly is the objection here? Why we would not be using well defined ops when we know there will be more users? Repeating what I said in the last thread, for io_uring it's used to implement the flow of buffers from userspace to the kernel, the ABI, which is orthogonal to the issue of what memory type it is and how it came there. And even if you mandate unnecessary dmabuf condoms for user memory in one form or another IMHO for no clear reason, the callbacks (or yet another if-else) would still be needed.
Sure, Mina can drop and hard code devmem path to easy the pain for him and delay the discussion, but then shortly after I will be re-sending same shit. So, what's the convincing argument _not_ to have it?
diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 92be1aaf18ccc..2cc986455bce6 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -557,8 +557,8 @@ netmem_ref page_pool_alloc_netmem(struct page_pool *pool, gfp_t gfp) return netmem;
/* Slow-path: cache empty, do real allocation */
if (static_branch_unlikely(&page_pool_mem_providers) && pool->mp_ops)
netmem = pool->mp_ops->alloc_pages(pool, gfp);
if (unlikely(page_pool_is_dmabuf(pool)))
netmem = mp_dmabuf_devmem_alloc_pages(): else netmem = __page_pool_alloc_pages_slow(pool, gfp); return netmem;