All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Danilo Krummrich" <dakr@kernel.org>
To: "Yury Norov" <ynorov@nvidia.com>
Cc: "Gary Guo" <gary@garyguo.net>,
	"Eliot Courtney" <ecourtney@nvidia.com>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Burak Emir" <burak.emir@gmail.com>,
	"Yury Norov" <yury.norov@gmail.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>,
	"Onur Özkan" <work@onurozkan.dev>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>, "Zhi Wang" <zhiw@nvidia.com>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	nova-gpu@lists.linux.dev, dri-devel@lists.freedesktop.org,
	dri-devel <dri-devel-bounces@lists.freedesktop.org>
Subject: Re: [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool
Date: Tue, 18 Aug 2026 15:55:57 +0200	[thread overview]
Message-ID: <DKS4DQA11LHT.22S0ACLYB9XAJ@kernel.org> (raw)
In-Reply-To: <aoON0FepAUw1b003@yury>

On Tue Aug 18, 2026 at 12:40 AM CEST, Yury Norov wrote:
> Again, any kernel API trusts it's caller. It holds for assembler,
> for C, and I don't see any reason why it shouldn't hold for Rust.

This argument is misleading in this context, because it is conflating "trusting
the caller" with "trusting the value".

The kernel's APIs generally do trust the caller (and even that does not always
hold), but they do not necessarily trust the arguments passed by a caller; this
entirely depends on the API contract.

And this makes a lot of sense; sometimes values originate from untrusted
sources, such as userspace.

But even if there is a clearly expressed API contract for the bounds of a value,
kernel APIs regularly do still check their validity.

For instance, do_mmap() does return -EINVAL if !len, despite the documentation
(*1) even saying:

	@len: The length of the mapping. Will be page-aligned and must be at
	      least 1 page in size.

In this case one reason is layering, the len argument *may* originate from
userspace, but it does not always originate from userspace (i.e. an untrusted
source).

So, if we'd write do_mmap() in Rust, it would be a perfect candidate for len
being NonZero.

This way there's only a single do_mmap() function, that doesn't need to bother
with a runtime check for len, because the argument already holds that invariant.

The way the invariant is obtained depends on the call site. If the value comes
from userspace, you do a fallible check

	let len = NonZero::new(len).ok_or(EINVAL)?;

If the value is known at compile time you can instead call

	let len = nz!(PAGE_SIZE);

which is checked at compile time.

Maybe you also already got a NonZero value from a different API that already
obtained the non-zero invariant that you just pass through.

It nicely separates the code that validates the value from the user of the
value, where the user of the value is only interested in the required invariant,
but not how the invariant is obtained.

IOW, do_mmap() does not care (and should not care) how the caller ensures that
len != 0.

> And undef isn't the case for alloc(0) - instead of making non-zero 'size' a
> part of API contract, we must make the function behavior well defined for this
> case.

We should only do this if the return value does not depend on an argument's
invariant, in which case no invariant is needed in the first place.

> kmalloc(0), for example, returns ZERO_SIZE_PTR. The pool.alloc_area(0) may
> return None, and probably trigger some warning.

That's because ZERO_SIZE_PTR *is* a useful return value that adds real value
(even more useful in Rust with ZST). It does for instance allow you to write:

	items = kmalloc_array(n, sizeof(*items), GFP_KERNEL);
	if (!items)
	    return -ENOMEM;

	for (i = 0; i < n; i++)
	    process(items[i]);

	kfree(items);

However, reserve_ids() is not like kmalloc(), a ChannelIdArea with a zero range
isn't useful at all.

And returning Result<Option<ChannelIdArea>> is not useful either, as it would
move the validation through NonZero (which you want to avoid) back to the user
now having to validate the Option instead, which, for obvious reasons, is worse
given that it is actually an error condition: there's nothing optional here,
it's just that the input argument was wrong, so Ok(None) is even misleading.

(*1) do_mmap()

I think the documentation is slighly misleading, since it says "must be at least
1 page in size", but the actual requirement is non-zero. It's just that anything
that is not page aligned is rounded up, so any 0 < len < PAGE_SIZE ends up at
PAGE_SIZE too.

  parent reply	other threads:[~2026-08-18 13:56 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12  8:51 [PATCH v5 0/5] rust: Add support for reserving of ranges of IDs Eliot Courtney
2026-08-12  8:51 ` [PATCH v5 1/5] rust: bitmap: use function-level cfg on kunit test Eliot Courtney
2026-08-12 22:23   ` Yury Norov
2026-08-12  8:51 ` [PATCH v5 2/5] rust: bitmap: restrict bitmap length to at most i32::MAX Eliot Courtney
2026-08-12 19:44   ` Yury Norov
2026-08-12  8:51 ` [PATCH v5 3/5] rust: bitmap: add contiguous area operations Eliot Courtney
2026-08-12 20:31   ` Yury Norov
2026-08-13  7:27     ` Eliot Courtney
2026-08-12  8:51 ` [PATCH v5 4/5] rust: id_pool: add contiguous area allocation Eliot Courtney
2026-08-12 21:16   ` Yury Norov
2026-08-13  7:29     ` Eliot Courtney
2026-08-12  8:51 ` [PATCH v5 5/5] gpu: nova-core: add ChannelIdPool Eliot Courtney
2026-08-12 22:18   ` Yury Norov
2026-08-13  7:31     ` Eliot Courtney
2026-08-13 18:32       ` Yury Norov
2026-08-13 20:20         ` Miguel Ojeda
2026-08-13 20:48         ` Danilo Krummrich
2026-08-13 20:58           ` Gary Guo
2026-08-13 21:38             ` John Hubbard
2026-08-13 21:44               ` Yury Norov
2026-08-13 21:53                 ` John Hubbard
2026-08-13 21:03           ` Yury Norov
2026-08-14  2:14             ` Eliot Courtney
2026-08-14  4:54               ` Eliot Courtney
2026-08-14  9:08                 ` Yury Norov
2026-08-14 14:53                   ` Yury Norov
2026-08-17  7:03                     ` Eliot Courtney
2026-08-17 10:54                       ` Gary Guo
2026-08-17 11:18                         ` Danilo Krummrich
2026-08-17 11:49                           ` Eliot Courtney
2026-08-17 12:16                             ` Danilo Krummrich
2026-08-17 12:37                               ` Eliot Courtney
2026-08-17 13:02                               ` Gary Guo
2026-08-17 14:08                                 ` Danilo Krummrich
2026-08-17 21:36                                   ` Burak Emir
2026-08-18  7:18                                     ` Miguel Ojeda
2026-08-17 22:40                                   ` Yury Norov
2026-08-18  6:41                                     ` Miguel Ojeda
2026-08-18 13:55                                     ` Danilo Krummrich [this message]
2026-08-17 20:20                       ` Yury Norov

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=DKS4DQA11LHT.22S0ACLYB9XAJ@kernel.org \
    --to=dakr@kernel.org \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=burak.emir@gmail.com \
    --cc=daniel.almeida@collabora.com \
    --cc=dri-devel-bounces@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ecourtney@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jhubbard@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nova-gpu@lists.linux.dev \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    --cc=work@onurozkan.dev \
    --cc=ynorov@nvidia.com \
    --cc=yury.norov@gmail.com \
    --cc=zhiw@nvidia.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.