From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="MLHbNY0C" Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0A9E5CD for ; Tue, 12 Dec 2023 17:09:51 -0800 (PST) Received: by mail-wm1-x32f.google.com with SMTP id 5b1f17b1804b1-40c32df9174so55231465e9.3 for ; Tue, 12 Dec 2023 17:09:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702429789; x=1703034589; 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=0i1d9QvoS4OxhYISeUTScW7akaFRcF7KdLSyFQ8hFdE=; b=MLHbNY0Czd3wPe32uDHEc/p1QFAuW5t37BjMpcSQ2JrBYEKt/Wqd1QbsTFzgLXsEN+ e3zrFyeJbLcDbesFlbaAbJ4AkhT9MSKbiUSpFqgK+n3Ihyw51fmrpHKKATAsVyY7MeGB fWBqPUvZmrKqbfVAVled2XVKP8ba27dIcf7oKvZ76EK+crAx7CGdVTPrRzYOlRNuhU7p pvX0DfSrYhSZXjFc5E5n9LKb5R4aBzuu2LPN/nbNqGeYnsUe4ftFMqYIqRFh+WScy9yx UD8XfTMfQyY0XR1pW3zgLXJ425d57zXmHsWNryMgdpTfLKmrQXK9O2funHSfzgsFvrjJ mWjw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702429789; x=1703034589; 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=0i1d9QvoS4OxhYISeUTScW7akaFRcF7KdLSyFQ8hFdE=; b=UA7hSUSUCqjiVFefbvQs10aVKNadYu8nx15VrCSlChxBE4YV34iHaQOxNbdOgRF77J I+nn2Oh33Rdke86/B4xyUXM02RSM7mBOtRyvLa4J8jSYv/diwHneZMRJeUGB87Z335Zc Y3tNH/QbGlwbPYtSmuWDLrmK93jVdMzRxksYGBHwPUU2SkSOfQmRA/7osRmgHcRDe77v sz12NjpItd9ZqzvAS2IGb1AZqe45AP3ukxAdcZFi3SCxhDV9GibP/13XflTiGi5mw300 KSwHr32TKWQI13hYFGJBfVcDXaZFxnia19gm0xUdn8w+hU5N9eEXdFffk3uwW57DfwbE I5Qg== X-Gm-Message-State: AOJu0Yw0qVL7bR9gylvT/fWHloU7MCBO4HFqEDGjeFTAKZ9D/g06hMe6 mSfZroij8/kePucr8QOyx7V49TMb1JTJDEFVUo7w5Q== X-Google-Smtp-Source: AGHT+IFHv6ulP97j2C3dKg3abBYpoQRGbAehnWd7YHUNtYhyRmrdi8aZpqMGubt6wRgBcXdnMdT0ZlS6rLmxwPeMcJk= X-Received: by 2002:a05:600c:4f86:b0:40c:48fb:ea01 with SMTP id n6-20020a05600c4f8600b0040c48fbea01mr1374675wmq.209.1702429789212; Tue, 12 Dec 2023 17:09:49 -0800 (PST) Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20231208005250.2910004-1-almasrymina@google.com> <20231208005250.2910004-9-almasrymina@google.com> <20231212122535.GA3029808@nvidia.com> <20231212143942.GF3014157@nvidia.com> <20231212150834.GI3014157@nvidia.com> In-Reply-To: <20231212150834.GI3014157@nvidia.com> From: Mina Almasry Date: Tue, 12 Dec 2023 17:09:35 -0800 Message-ID: Subject: Re: [net-next v1 08/16] memory-provider: dmabuf devmem memory provider To: Jason Gunthorpe Cc: Shailend Chand , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-arch@vger.kernel.org, linux-kselftest@vger.kernel.org, bpf@vger.kernel.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Jonathan Corbet , Jeroen de Borst , Praveen Kaligineedi , Jesper Dangaard Brouer , Ilias Apalodimas , Arnd Bergmann , David Ahern , Willem de Bruijn , Shuah Khan , Sumit Semwal , =?UTF-8?Q?Christian_K=C3=B6nig?= , Yunsheng Lin , Harshitha Ramamurthy , Shakeel Butt , Willem de Bruijn , Kaiyuan Zhang , Christoph Hellwig Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Dec 12, 2023 at 7:08=E2=80=AFAM Jason Gunthorpe wr= ote: > > On Tue, Dec 12, 2023 at 06:58:17AM -0800, Mina Almasry wrote: > > > Jason, we set the LSB on page_pool_iov pointers before casting it to > > struct page pointers. The resulting pointers are not useable as page > > pointers at all. > > I understand that, the second ask is about maintainability of the mm > by using correct types. > > > > Perhaps you can simply avoid this by arranging for this driver to als= o > > > exclusively use some special type to indicate the dual nature of the > > > pointer and leave the other drivers as using the struct page version. > > > > This is certainly possible, but it requires us to rename all the page > > pointers in the page_pool to the new type, and requires the driver > > adding devmem TCP support to rename all the page* pointer instances to > > the new type. It's possible but it introduces lots of code churn. Is > > the LSB + cast not a reasonable compromise here? I feel like the trick > > of setting the least significant bit on a pointer to indicate it's > > something else has a fair amount of precedent in the kernel. > > Linus himself has complained about exactly this before, and written a cle= anup: > > https://lore.kernel.org/linux-mm/20221108194139.57604-1-torvalds@linux-fo= undation.org/ > > If you mangle a pointer *so it is no longer a pointer* then give it a > proper opaque type so the compiler can check everything statically and > require that the necessary converters are called in all cases. > > You call it churn, I call it future maintainability. :( > > No objection to using the LSB, just properly type a LSB mangled > pointer so everyone knows what is going on and don't call it MM's > struct page *. > > I would say this is important here because it is a large driver facing > API surface. > OK, I imagine this is not that hard to implement - it's really whether the change is acceptable to reviewers. I figure I can start by implementing a no-op abstraction to page*: typedef struct page netmem_t and replace the page* in the following places with netmem_t*: 1. page_pool API (not internals) 2. drivers using the page_pool. 3. skb_frag_t. I think that change needs to be a separate series by itself. Then the devmem patches would on top of that change netmem_t such that it can be a union between struct page and page_pool_iov and add the special handling of page_pool_iov. Does this sound reasonable? -- Thanks, Mina