From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D28E516E863 for ; Tue, 7 May 2024 16:42:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715100143; cv=none; b=pKTnUjnrIUpdRZLFvQCyvv5FCKZh//4nszqNneUpg6bCo7lyJCfbA41LrqDXWMXqwMUaLApVJCzRy+cqoefog+R+xjmh7lfD5t6Ius0Q87Fn72dsHqDLEBSTbVkyMIAC2QoBC/412atqge2Klo/yj9NnZxeGFg7nz2MkqqTDYPY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1715100143; c=relaxed/simple; bh=4xUNxPDzztu8L8/w3Kr2Lgr1+A9SAEy6b6FHPJHwwv4=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=dfQANFULfqmSJJApqhi33dL1vrQOtKS0Ls4v5/ZAiUjKJz1C3GePcCgCoU9rOdkZ0UqcKd53mtT8Y83f2OPyk5/vANxns9gpWw8Z9ERQj2wyUl7PI2vx1MbbSZqvH32E+uNQYKV9/GLKOsJThMnI0oOFw+hNHNDbqR1S4iGiiS4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=fFMNv4LT; arc=none smtp.client-ip=209.85.218.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="fFMNv4LT" Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-a59c448b44aso657062366b.2 for ; Tue, 07 May 2024 09:42:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1715100138; x=1715704938; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=dI6bl9uqwH7zmPDcLL9svN5oVbQ7KUvxyIQRaOdtpl4=; b=fFMNv4LTPJ31z4uW9YnTXqY/+P/Lu3XxPsX1vj/FcwrlahaksKtwSIJkvvDzcu2rIw kUFK/asO0I1jpjKDr04+deUss1rHngscpNh095cNT9yAlUJGoBVdvFddfEUhojuVSLd8 Eto5no1o3YDn7s96fv4R3s6xbKb5Bsur/LC9clY8PjSwiL/9uzZj5LS5/oC+2OGW3A0+ vm7S0Y+TFMGs2VB53VFjDniE4wLpnem+YNJTJmc4qzG2ipR7VsHrwzdTNO1UY3pswfMu NQONvfepGVrBaio6afuQzEqvzCXU8mh+EjojwyPooBi4z5e68SfByNYD/7cU59orv47R zBcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715100138; x=1715704938; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=dI6bl9uqwH7zmPDcLL9svN5oVbQ7KUvxyIQRaOdtpl4=; b=sLy9s1tnQ/f0TOXbkwxzoAfKH/cjE3ZHtTruiHRjr13A3/oTnYhL/0Kcy2O7vklWKm e4ecoSjTVSoedDlNgC6X7YYXVp6yguYwvz9jRilmST/EnzCbL11HjWxC9cFHAmOodADN aDJIRnzGA9wNWNi9I6mznAINcaBQVGU/LNqh6UJXicfJEg6hKnxu9czLVlLP+e2d7iyX Fn35iwLt9lB4rWNWhNndryAB+wRTa2aUJp8ZEjXhU2e6d08zjhzK54TwnFTUQjeg+kWS Yw4+ImKkM5ZfXHbmKOlq0Eu6dfLPih9wMmQQDw/SYO5YgbIZyw9Nd6bqCJ3DW6YhIgF9 mwPw== X-Forwarded-Encrypted: i=1; AJvYcCXRej196nMCM7W+c3t37otHODkCWpXNYil94zuZRnOWH575iVQfUSEqcv0HD8DdqSMs6FofaWrgMDfqUQjOOBtzD9apCtSh24LWr2I= X-Gm-Message-State: AOJu0YxLv8oTteoK6mfGA/QCIBZZDC+fFYPUrVF71r6DhOX3yjGb38k5 KoARKslNR3SZErKjLtaFG7MXd/b1Brx3roRA4jE7m65d07di19brYM5q8oO7UWu2R3MXSuFJmia oHFZVhHtEa9JjsE7lCFbAxYeNwSI1KSobq/wG X-Google-Smtp-Source: AGHT+IGXrfCrbPuBFYGXmT+FZdafT4oz7XFNMT8FfUWaWmudph1z0+uFgT86I0JhwB1+1uK0PlezLiQ7rsxPPg9MW2Q= X-Received: by 2002:a17:906:1d4b:b0:a59:c833:d275 with SMTP id a640c23a62f3a-a59fb94dbe4mr1458266b.30.1715100137853; Tue, 07 May 2024 09:42:17 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-alpha@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20240403002053.2376017-1-almasrymina@google.com> <20240403002053.2376017-3-almasrymina@google.com> <20b1c2d9-0b37-414c-b348-89684c0c0998@gmail.com> <20240507161857.GA4718@ziepe.ca> In-Reply-To: From: Mina Almasry Date: Tue, 7 May 2024 09:42:05 -0700 Message-ID: Subject: Re: [RFC PATCH net-next v8 02/14] net: page_pool: create hooks for custom page providers To: Christoph Hellwig Cc: Jason Gunthorpe , Pavel Begunkov , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-alpha@vger.kernel.org, linux-mips@vger.kernel.org, linux-parisc@vger.kernel.org, sparclinux@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-arch@vger.kernel.org, bpf@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Jonathan Corbet , Richard Henderson , Ivan Kokshaysky , Matt Turner , Thomas Bogendoerfer , "James E.J. Bottomley" , Helge Deller , Andreas Larsson , Jesper Dangaard Brouer , Ilias Apalodimas , Steven Rostedt , Masami Hiramatsu , Mathieu Desnoyers , Arnd Bergmann , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Eduard Zingerman , Song Liu , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Steffen Klassert , Herbert Xu , David Ahern , Willem de Bruijn , Shuah Khan , Sumit Semwal , =?UTF-8?Q?Christian_K=C3=B6nig?= , Amritha Nambiar , Maciej Fijalkowski , Alexander Mikhalitsyn , Kaiyuan Zhang , Christian Brauner , Simon Horman , David Howells , Florian Westphal , Yunsheng Lin , Kuniyuki Iwashima , Jens Axboe , Arseniy Krasnov , Aleksander Lobakin , Michael Lass , Jiri Pirko , Sebastian Andrzej Siewior , Lorenzo Bianconi , Richard Gobert , Sridhar Samudrala , Xuan Zhuo , Johannes Berg , Abel Wu , Breno Leitao , David Wei , Shailend Chand , Harshitha Ramamurthy , Shakeel Butt , Jeroen de Borst , Praveen Kaligineedi Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, May 7, 2024 at 9:24=E2=80=AFAM Christoph Hellwig wrote: > > On Tue, May 07, 2024 at 01:18:57PM -0300, Jason Gunthorpe wrote: > > On Tue, May 07, 2024 at 05:05:12PM +0100, Pavel Begunkov wrote: > > > > even in tree if you give them enough rope, and they should not have > > > > that rope when the only sensible options are page/folio based kerne= l > > > > memory (incuding large/huge folios) and dmabuf. > > > > > > I believe there is at least one deep confusion here, considering you > > > previously mentioned Keith's pre-mapping patches. The "hooks" are not > > > that about in what format you pass memory, it's arguably the least > > > interesting part for page pool, more or less it'd circulate whatever > > > is given. It's more of how to have a better control over buffer lifet= ime > > > and implement a buffer pool passing data to users and empty buffers > > > back. > > > > Isn't that more or less exactly what dmabuf is? Why do you need > > another almost dma-buf thing for another project? > > That's the exact point I've been making since the last round of > the series. We don't need to reinvent dmabuf poorly in every > subsystem, but instead fix the odd parts in it and make it suitable > for everyone. > FWIW the change Christoph is requesting is straight forward from my POV and doesn't really hurt the devmem use case. I'd basically remove the ops and add an if statement in the slow path where the ops are being used to alloc/free from dmabuf instead of alloc_pages(). Something like (very rough, doesn't compile): 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_op= s) - netmem =3D pool->mp_ops->alloc_pages(pool, gfp); + if (page_pool_is_dmabuf(pool)) + netmem =3D mp_dmabuf_devmem_alloc_pages(): else netmem =3D __page_pool_alloc_pages_slow(pool, gfp); return netmem; The folks that will be negatively impacted by this are Jakub/Pavel/David. I think all were planning to extend the hooks for io_uring or other memory types. Pavel/David, AFAICT you have these options here (but maybe you can think of more): 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. 2. Follow the example of devmem TCP and add another if statement to alloc from io_uring, so something like: diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 92be1aaf18ccc..3545bb82c7d05 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -557,8 +557,10 @@ 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_op= s) - netmem =3D pool->mp_ops->alloc_pages(pool, gfp); + if (page_pool_is_dmabuf(pool)) + netmem =3D mp_dmabuf_devmem_alloc_pages(): + else if (page_pool_is_io_uring(pool)) + netmem =3D mp_io_uring_alloc_pages(): else netmem =3D __page_pool_alloc_pages_slow(pool, gfp); return netmem; Note that Christoph/Jason may not like you adding non-dmabuf io_uring backing memory in the first place, so there may be pushback against this approach. 3. Pushback on the nack on this thread. It seems you're already discussing this. I'll see what happens. To be honest the GVE queue-API has just been merged I think, so I'm now unblocked on sending non-RFCs of this work and I'm hoping to send the next version soon. I may apply these changes on the next version for more discussion or leave as is and carry the nack until the conversation converges. --=20 Thanks, Mina