Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Tue, 12 Dec 2023 11:08:34 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Mina Almasry <almasrymina@google.com>
Cc:     Shailend Chand <shailend@google.com>, 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" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Jeroen de Borst <jeroendb@google.com>,
        Praveen Kaligineedi <pkaligineedi@google.com>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        Ilias Apalodimas <ilias.apalodimas@linaro.org>,
        Arnd Bergmann <arnd@arndb.de>,
        David Ahern <dsahern@kernel.org>,
        Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
        Shuah Khan <shuah@kernel.org>,
        Sumit Semwal <sumit.semwal@linaro.org>,
        Christian =?utf-8?B?S8O2bmln?= <christian.koenig@amd.com>,
        Yunsheng Lin <linyunsheng@huawei.com>,
        Harshitha Ramamurthy <hramamurthy@google.com>,
        Shakeel Butt <shakeelb@google.com>,
        Willem de Bruijn <willemb@google.com>,
        Kaiyuan Zhang <kaiyuanz@google.com>,
        Christoph Hellwig <hch@infradead.org>
Subject: Re: [net-next v1 08/16] memory-provider: dmabuf devmem memory
 provider
Message-ID: <20231212150834.GI3014157@nvidia.com>
References: <20231208005250.2910004-1-almasrymina@google.com>
 <20231208005250.2910004-9-almasrymina@google.com>
 <20231212122535.GA3029808@nvidia.com>
 <CAHS8izMVMx0fpT=dWsnD7piqs1g7Fam8Xf5dK3iOFNxeOQD9vQ@mail.gmail.com>
 <20231212143942.GF3014157@nvidia.com>
 <CAHS8izNHtemjjkMf43grCHP1RZ=2UFiMtgea0M6+PaAgC=DDMQ@mail.gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAHS8izNHtemjjkMf43grCHP1RZ=2UFiMtgea0M6+PaAgC=DDMQ@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?mrEyRSkTR3gUgnuY1J2tdZGe/P6rs8W4YBEfEQXcPcFgl07D/PbU8F41j7N/?=
 =?us-ascii?Q?FGywTcY5ZQUsUmzEFgMSJtM8gFs1aiOcVWhSg+GPrGsaSnaLBqN2KeV5DzDL?=
 =?us-ascii?Q?41IyPeJ9TgVJLscS90sRseRwgskTN713sA9QSLX4445rje2qUzCYwPIuG+lv?=
 =?us-ascii?Q?8UZL5CNYr0w64Zj1aswGb8v9fmHI7L3GlUHynBaZQvZymNIWWc09bW4XrbMX?=
 =?us-ascii?Q?xwzXU8yzekOMtuaQk/gF1kowRA5gx7zRJCZs6+gS3a7smAwHZOZfDaK+Tj5Q?=
 =?us-ascii?Q?Mxm0TMV/2U0QFLzgPYs4TjNe8zeSAzulLpGhN6ZoYDqrmR/C18wipHE59MxW?=
 =?us-ascii?Q?k8iFIsN6sCwWPbWQytdPRk8yP0dbuH7LsBA/R5eNM7kR0g29ZoXGAgjZsrIF?=
 =?us-ascii?Q?VSNCBJReQtRfHhfyv5UgVmBVE47zC7x/RuJizrxICxvt1t7J/N8U203NFhpA?=
 =?us-ascii?Q?Y1Q4IT2fGGMD1oXOXExekl3D9QyseQ4Ao1BbHG8qy0H8VDS+WAo/hz2llkfa?=
 =?us-ascii?Q?VN7QKS6GiU7FXMg6iktyewheQLX0261Vac2TjLTfGnMRBIccWyRI3gIUJbKm?=
 =?us-ascii?Q?Xe2OXoFuV7ny7rMgJbg77gfCxroqRODDEUj/ZATk5qW6MsyxMCIh7VvhaONa?=
 =?us-ascii?Q?b3ivJAG33hUA5LLpLksPjAQRRK4WwWeMqAaPUz5hg6HfhSN+PpkfWCtIY03g?=
 =?us-ascii?Q?jE4ZrY88iTzBMCsiJRp+w/jh1gUG7c0BFlnJeF6TALqsvZgF02A6N+aicIIx?=
 =?us-ascii?Q?dxfnTRNXxBJgOoGBvLZdrhpPVPVsnFDDqtCb751rQhJ5OfmkF15PZATAjJJp?=
 =?us-ascii?Q?nYnEDPWKGqPfTV2Wba4DODihvuUgbOHa5yHuGSJWCry+QY1UUeAN6enWk2IB?=
 =?us-ascii?Q?CLCmHZomsquft7dIDVuQYiNlOgR4++Bv59LphBhMG+xb8DVOQGSMvPj1vhET?=
 =?us-ascii?Q?VS0ZdNgN45O9vLYkkveSqCrbRFDTA1p4emQFHdchHfJ4iEYvnUSkplNBVOew?=
 =?us-ascii?Q?bHtAwKCTWABaGz1t6fVu7RA5QQqHWoJcR8S9/MCp9WFruHRXi85OUHErgKnh?=
 =?us-ascii?Q?Mr/JU4qYTEuEw8W4J9NQUW7wxNEbmHQ+/b0kO2ypB5qr8JfqlYvCLu3P0V7M?=
 =?us-ascii?Q?7AIUrHHQjetxDmzGtp+LONn4KnC4EmUHRkOniZJpGWPgAFyrWmsn3Ajrx9Ey?=
 =?us-ascii?Q?lkbzq5l1Fe1iDWyXsxIMKAZZLnXp10HzTalqJ7TcGHFIV1gtWB9JjLTGYU4f?=
 =?us-ascii?Q?SQN+DJOyrqcKRv1GhD0EBo21R08lGVdLVLtQ4tFpbDMA4erUotcGVL/t76cs?=
 =?us-ascii?Q?jk1scj27LnjchzJFChyyynn+2QhlX8beSWCe2J6CuQFRRca9NxPLuYCaxF2o?=
 =?us-ascii?Q?pZ1mf/OTeAhkLljGT1YDtpQEBH+F0RbvOzgdbVp7as6fTx2k8frB9IDkfCxS?=
 =?us-ascii?Q?xoQNMEp6BL5o+Ul2WAYtQoaCTrysScjvCoGTBDB599FhLGO4YAfG5SxxVg+R?=
 =?us-ascii?Q?L1DPU5oEMm7Te3tguCLYmdENqRk8w/wuGEF2iZajLXxCvzw06nlPvNK3HBxf?=
 =?us-ascii?Q?dzyMvzDiKKDe3XdHmO59yQYeaZX77P8phcd8FfT9?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f7e77963-7a4e-47b7-5674-08dbfb24365a
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Dec 2023 15:08:35.3779
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: JWLKQFKjSNhxNlo+bcKSshFN7zqiaPkLLotEj/GU512i4fMIL6yPfhGNEQwhFm54
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5722
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 12 Dec 2023 07:08:46 -0800 (PST)

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 also
> > 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 cleanup:

https://lore.kernel.org/linux-mm/20221108194139.57604-1-torvalds@linux-foundation.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.

Jason