public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Aditya Rajan" <adi.dev.github@gmail.com>,
	"Gary Guo" <gary@garyguo.net>, <dakr@kernel.org>,
	<ojeda@kernel.org>
Cc: <abdiel.janulgue@gmail.com>, <daniel.almeida@collabora.com>,
	<robin.murphy@arm.com>, <a.hindborg@kernel.org>,
	<boqun@kernel.org>, <bjorn3_gh@protonmail.com>,
	<lossin@kernel.org>, <aliceryhl@google.com>, <tmgross@umich.edu>,
	<driver-core@lists.linux.dev>, <rust-for-linux@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] rust: dma: return EOVERFLOW instead of ENOMEM on size overflow
Date: Sat, 04 Apr 2026 21:13:41 +0100	[thread overview]
Message-ID: <DHKN8U9VHH45.1ONRTFLAVID9D@garyguo.net> (raw)
In-Reply-To: <DHKJNOFD1P03.15QMM3BFH0KLB@gmail.com>

On Sat Apr 4, 2026 at 6:24 PM BST, Aditya Rajan wrote:
> On Sat Apr 4, 2026 at 6:15 AM PDT, Gary Guo wrote:
>
>> Thanks for the patch, but the behaviour here is intended.
>>
>> Neither our `KVec` implementation nor upstream Rust distinguishes between
>> allocation error caused by array size exceeding address space or running out of
>> memory to allocate (`AllocError` is returned and it converts to ENOMEM).
>>
>> `kmalloc_array` also just returns `NULL` when overflows, so arguably this
>> behaviour also aligns us with C side.
>>
>> Abstractly, the system is indeed running out memory because it cannot allocate
>> something larger than its address space.
>
> Hi Gary,
>
> Thanks for the reply, I saw at some similar places where EOVERFLOW is used,
> that is why i thought we should change this error code:
>
> * In nouveau_drv.h, `u_memcpya()` does `check_mul_overflow(nmemb, size,
> &bytes)` and returns ERR_PTR(-EOVERFLOW), it is kind of same multiplication
> overflow on `nmemb*size` before an allocation. Similarly `mm/mmap.c` returns
> EOVERFLOW for arithmetic overflow in offset calculations, it also has a
> comment `/* offset overflow? */`.

I think these cases are all related to values ultimately controlled by
userspace, and the error conditions are propagated back to userspace, so it
makes more sense to distinguish errors.

On the other hand, for KVec/Coherent cases, it is less useful to distinguish
between the cases. If we want to add EOVERFLOW, then we'd need to extend
AllocError to have two enums, which I wouldn't be against, but I think it's not
worth having.

(I wanted Coherent to use AllocError as error too, but as it does not support
zero-size allocation, so I didn't change the error type. I guess one option is
to handle zero-size case by using a dangling pointer similar to `KVec`.)

>
> * Also I saw existing Rust kernel code already follows similar convention, see
> `rust/kernel/uaccess.rs` it uses `offset.checked_add(count).ok_or(EOVERFLOW)?`
> for the same kind of arithmetic overflow check.
>
> * For `kmalloc_array` i think it conflates overflow with OOM because its
> return type (pointer) can't express distinct errors, maybe it should be
> improved as well ?. When the API can distinguish (like here, or in nouveau),
> the kernel does use (or maybe should use?) `EOVERFLOW`.
>
> IMO two failures have different semantics for callers, ENOMEM is transient
> (retry may succeed under less memory pressure), EOVERFLOW is deterministic
> (the input will never work). Using ENOMEM for overflow could mislead a caller
> into retrying a request that can never succeed.

If I allocate `usize::MAX`, it'll never work but the error would still be
ENOMEM, and I see no reason to differentiate `usize::MAX` and `usize::MAX + 1`.

Best,
Gary

  parent reply	other threads:[~2026-04-04 20:13 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-03 21:28 [PATCH] rust: dma: return EOVERFLOW instead of ENOMEM on size overflow Aditya Rajan
2026-04-04 13:15 ` Gary Guo
2026-04-04 17:24   ` Aditya Rajan
2026-04-04 19:43     ` Danilo Krummrich
2026-04-04 20:13     ` Gary Guo [this message]
2026-04-04 20:28       ` Danilo Krummrich
2026-04-05  9:51 ` kernel test robot

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=DHKN8U9VHH45.1ONRTFLAVID9D@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=abdiel.janulgue@gmail.com \
    --cc=adi.dev.github@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=driver-core@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=robin.murphy@arm.com \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    /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