rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Joel Fernandes" <joelagnelf@nvidia.com>
Cc: "Alice Ryhl" <aliceryhl@google.com>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	dri-devel@lists.freedesktop.org, linux-pci@vger.kernel.org
Subject: Re: [PATCH] rust: io: use const generics for read/write offsets
Date: Fri, 19 Sep 2025 01:26:28 +0200	[thread overview]
Message-ID: <DCWBCL9U0IY4.NFNUMLRULAWM@kernel.org> (raw)
In-Reply-To: <20250918181357.GA1825487@joelbox2>

On Thu Sep 18, 2025 at 8:13 PM CEST, Joel Fernandes wrote:
> On Thu, Sep 18, 2025 at 03:02:11PM +0000, Alice Ryhl wrote:
>> Using build_assert! to assert that offsets are in bounds is really
>> fragile and likely to result in spurious and hard-to-debug build
>> failures. Therefore, build_assert! should be avoided for this case.
>> Thus, update the code to perform the check in const evaluation instead.
>
> I really don't think this patch is a good idea (and nobody I spoke to thinks
> so). Not only does it mess up the user's caller syntax completely, it is also

I appreacite you raising the concern, but I rather have other people speak up
themselves.

> super confusing to pass both a generic and a function argument separately.

Why? We assert that the offset has to be const, whereas the value does not
have this requirement, so this makes perfect sense to me.

(I agree that it can look unfamiliar at the first glance though.)

> Sorry if I knew this would be the syntax, I would have objected even when we
> spoke :)
>
> I think the best fix (from any I've seen so far), is to move the bindings
> calls of offending code into a closure and call the closure directly, as I
> posted in the other thread. I also passed the closure idea by Gary and he
> confirmed the compiler should behave correctly (I will check the code gen
> with/without later). Gary also provided a brilliant suggestion that we can
> call the closure directly instead of assigning it to a variable first. That
> fix is also smaller, and does not screw up the users. APIs should fix issues
> within them instead of relying on user to work around them.

This is not a workaround, this is an idiomatic solution (which I probably should
have been doing already when I introduced the I/O code).

We do exactly the same thing for DmaMask::new() [1] and we agreed on doing the
same thing for Alignment as well [2].

[1] https://rust.docs.kernel.org/kernel/dma/struct.DmaMask.html#method.new
[2] https://lore.kernel.org/rust-for-linux/20250908-num-v5-1-c0f2f681ea96@nvidia.com/

  reply	other threads:[~2025-09-18 23:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-18 15:02 [PATCH] rust: io: use const generics for read/write offsets Alice Ryhl
2025-09-18 18:13 ` Joel Fernandes
2025-09-18 23:26   ` Danilo Krummrich [this message]
2025-09-19  7:59     ` Joel Fernandes
2025-09-19  9:26       ` Benno Lossin
2025-09-19 20:53         ` Joel Fernandes
2025-09-22  6:25           ` Alexandre Courbot
2025-09-19 20:56     ` Gary Guo
2025-09-19 23:56       ` Danilo Krummrich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DCWBCL9U0IY4.NFNUMLRULAWM@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=joelagnelf@nvidia.com \
    --cc=kwilczynski@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rafael@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).