From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ej1-f51.google.com (mail-ej1-f51.google.com [209.85.218.51]) (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 9E7B11863 for ; Sun, 17 Dec 2023 08:15:15 +0000 (UTC) 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="1yKWnsc1" Received: by mail-ej1-f51.google.com with SMTP id a640c23a62f3a-a1d2f89ddabso228540066b.1 for ; Sun, 17 Dec 2023 00:15:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1702800914; x=1703405714; 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=60F2ATzAHDKtGPRs2bXfsavW1DI+agjrMkffzWa7PLI=; b=1yKWnsc1BiM/fv7trNG8kQ87EY1/JstLrlFwfBUvc8OjDg99cqspvqkcyJmMR0E2Pu IBJIrdcVKB5XxXsUk6i/F0oYY6WcMflL6QCk8LIc/B7TYvyro6ZyCJw/BNkc6yeU9AIr bcgYYadSydATY2Q6C2bquuaACBsQjmCFVakZdM1RdSvC+I5J0KovIKZr85wILy8CTh7o H43/pn7pKNv3RlApzXykOOpd26Qqj8wi9xly5QkzpnLBD1/8akurJMcAb4Q099CSyZB5 2mzQbdyJIukEzllzBRNIucH8pBvhpL9mxTmGlFfZ7zAxQMHv+W46mwV+ibRVxyb3SZtq lx+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1702800914; x=1703405714; 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=60F2ATzAHDKtGPRs2bXfsavW1DI+agjrMkffzWa7PLI=; b=rs98vt1LZRpkOSx0YfBC7W7/1tc1U/kIVmKwcwe7Odle98UgThbivVGma7Oljq/grD R5eYEoDfjoXfEiWOtj0U9gGNLS9YkBWWx//KTg8gvmaqwyK13Y7WNgHkF3pT0oKsgPEg 9dxscNABTmqDj2l9ejMjo29/XElyiptXa584JrYsvNUR7luVI9HWRjsm3d9RXnxGESJO FPa2WT05iTff70RyWUJep2Zo+e1MKi0K5ta9SjPscfX6aTpBarvg3+3nGlw3mDVvGLt0 J4xH18E2rGReZ3Z/Aa2EcqnTxZVuwYhXjrGOuo0YNzGtNOsR0YZJ1dhbhRIG4miRLZ+n IgSw== X-Gm-Message-State: AOJu0YwMOdui4+N19cD4p+SWHk9/qJmEYiHeAn8DvavQlADPt6lThy9x L65LLikhqwaqWepbXDsdDu9Gf2Rkab+k2HWnBPlrg013gdmjjgu3OilPdw== X-Google-Smtp-Source: AGHT+IHQrLXo/6D4cvtJ3HR0M3jIIjNIvh9V5qDlot7YQKT/hN6uzbLZJBzF40qS8xdCqHfqb16F0Y3coMCmYjXpSBA= X-Received: by 2002:a17:906:8f:b0:a23:26bf:6cd9 with SMTP id 15-20020a170906008f00b00a2326bf6cd9mr925030ejc.34.1702800913483; Sun, 17 Dec 2023 00:15:13 -0800 (PST) Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20231214020530.2267499-1-almasrymina@google.com> <20231214020530.2267499-3-almasrymina@google.com> <20231215185159.7bada9a7@kernel.org> <84787af3-aa5e-4202-8578-7a9f14283d87@kernel.org> In-Reply-To: <84787af3-aa5e-4202-8578-7a9f14283d87@kernel.org> From: Mina Almasry Date: Sun, 17 Dec 2023 00:14:59 -0800 Message-ID: Subject: Re: [RFC PATCH net-next v1 2/4] net: introduce abstraction for network memory To: David Ahern Cc: Jakub Kicinski , linux-kernel@vger.kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org, Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Greg Kroah-Hartman , "Rafael J. Wysocki" , Sumit Semwal , =?UTF-8?Q?Christian_K=C3=B6nig?= , Michael Chan , "David S. Miller" , Eric Dumazet , Paolo Abeni , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , Wei Fang , Shenwei Wang , Clark Wang , NXP Linux Team , Jeroen de Borst , Praveen Kaligineedi , Shailend Chand , Yisen Zhuang , Salil Mehta , Jesse Brandeburg , Tony Nguyen , Thomas Petazzoni , Marcin Wojtas , Russell King , Sunil Goutham , Geetha sowjanya , Subbaraya Sundeep , hariprasad , Felix Fietkau , John Crispin , Sean Wang , Mark Lee , Lorenzo Bianconi , Matthias Brugger , AngeloGioacchino Del Regno , Saeed Mahameed , Leon Romanovsky , Horatiu Vultur , UNGLinuxDriver@microchip.com, "K. Y. Srinivasan" , Haiyang Zhang , Wei Liu , Dexuan Cui , Jassi Brar , Ilias Apalodimas , Alexandre Torgue , Jose Abreu , Maxime Coquelin , Siddharth Vadapalli , Ravi Gunasekaran , Roger Quadros , Jiawen Wu , Mengyuan Lou , Ronak Doshi , VMware PV-Drivers Reviewers , Ryder Lee , Shayne Chen , Kalle Valo , Juergen Gross , Stefano Stabellini , Oleksandr Tyshchenko , Andrii Nakryiko , Martin KaFai Lau , Song Liu , Yonghong Song , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa , Stefan Hajnoczi , Stefano Garzarella , Shuah Khan , =?UTF-8?B?TWlja2HDq2wgU2FsYcO8bg==?= , Nathan Chancellor , Nick Desaulniers , Bill Wendling , Justin Stitt , Jason Gunthorpe , Shakeel Butt , Yunsheng Lin , Willem de Bruijn Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sat, Dec 16, 2023 at 5:45=E2=80=AFPM David Ahern wr= ote: > > On 12/16/23 2:10 PM, Mina Almasry wrote: > > On Fri, Dec 15, 2023 at 6:52=E2=80=AFPM Jakub Kicinski wrote: > >> > >> On Wed, 13 Dec 2023 18:05:25 -0800 Mina Almasry wrote: > >>> +struct netmem { > >>> + union { > >>> + struct page page; > >>> + > >>> + /* Stub to prevent compiler implicitly converting from = page* > >>> + * to netmem_t* and vice versa. > >>> + * > >>> + * Other memory type(s) net stack would like to support > >>> + * can be added to this union. > >>> + */ > >>> + void *addr; > >>> + }; > >>> +}; > >> > >> My mind went to something like: > >> > >> typedef unsigned long __bitwise netmem_ref; > >> > >> instead. struct netmem does not exist, it's a handle which has > >> to be converted to a real type using a helper. > > > > Sure thing I can do that. Is it better to do something like: > > > > struct netmem_ref; > > > > like in this patch: > > > > https://lore.kernel.org/linux-mm/20221108194139.57604-1-torvalds@linux-= foundation.org/ > > > > Asking because checkpatch tells me not to add typedefs to the kernel, > > but checkpatch can be ignored if you think it's OK. > > > > Also with this approach I can't use container_of and I need to do a > > cast, I assume that's fine. > > > > Isn't that the whole point of this set - to introduce a new data type > and avoid casts? My understanding here the requirements from Jason are: 1. Never pass a non-page to an mm api. 2. If a mangle a pointer to indicate it's not a page, then I must not call it mm's struct page*, I must add a new type. I think both requirements are met regardless of whether netmem_to_page() is implemented using union/container_of or straight casts. folios implemented something similar being unioned with struct page to avoid casts. I honestly could go either way on this. The union provides some self documenting code and avoids casts. The implementation without the union obfuscates the type and makes it much more opaque. I finished addressing the rest of the comments and I have this series and the next devmem TCP series ready to go, so I fired v2 of this patchset. If one feels strongly about this let me know and I will re-spin. --=20 Thanks, Mina