public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "John Hubbard" <jhubbard@nvidia.com>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Gary Guo" <gary@garyguo.net>
Cc: "Alexandre Courbot" <acourbot@nvidia.com>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Eliot Courtney" <ecourtney@nvidia.com>,
	"Zhi Wang" <zhiw@nvidia.com>, "David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	nouveau@lists.freedesktop.org, rust-for-linux@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v5 21/38] rust: ptr: add const_align_up() and enable inline_const feature
Date: Wed, 04 Mar 2026 11:18:02 +0000	[thread overview]
Message-ID: <DGTYFUCCECHS.T8EW6ZPL8NZS@garyguo.net> (raw)
In-Reply-To: <282c94d3-f32c-4851-969f-61fc968de6a7@nvidia.com>

On Wed Mar 4, 2026 at 3:47 AM GMT, John Hubbard wrote:
> On 2/23/26 6:20 AM, Danilo Krummrich wrote:
>> On Mon Feb 23, 2026 at 3:16 PM CET, Gary Guo wrote:
>>> On 2026-02-23 11:07, Danilo Krummrich wrote:
>>>> On Sun Feb 22, 2026 at 8:04 PM CET, John Hubbard wrote:
>>>>> On 2/21/26 11:46 PM, Gary Guo wrote:
>>>>>> On 2026-02-21 02:09, John Hubbard wrote:
>>>>>>> Add const_align_up<ALIGN>() to kernel::ptr as the const-compatible
>>>>>>> equivalent of Alignable::align_up(). This uses inline_const to validate
>>>>>>> the alignment at compile time with a clear error message.
>>>>>>>
>>>>> ...
>>>>>
>>>>>>> +#[inline(always)]
>>>>>>> +pub const fn const_align_up<const ALIGN: usize>(value: usize) -> usize {
>>>>>>> +    const { assert!(ALIGN.is_power_of_two(), "ALIGN must be a power of two") };
>>>>>>> +    match value.checked_add(ALIGN - 1) {
>>>>>>> +        Some(v) => v & !(ALIGN - 1),
>>>>>>> +        None => panic!("const_align_up: overflow"),
>>>>>>
>>>>>> This is wrong. Either this function is always used in const context, in which case
>>>>>> you take `ALIGN` as normal function parameter and use `build_assert` and `build_error`,
>>>>>> or this function can be called from runtime and you shouldn't have a panic call here.
>>>>
>>>> I think the most common case is that ALIGN is const, but value is not.
>>>>
>>>> What about keeping the function as is (with the panic() replaced with a Result)
>>>> and also add
>>>>
>>>> 	#[inline(always)]
>>>> 	pub const fn const_expect<T: Copy>(opt: Result<T>, &'static str) -> T {
>>>> 	    match opt {
>>>> 	        Ok(v) => v,
>>>> 	        Err(_) => panic!(""),
>>>> 	    }
>>>> 	}
>>>>
>>>
>>> We already have `Alignable::align_up` for non-const cases, so this would only be used
>>> in const context and I don't see the need of having explicit const_expect?
>> 
>> Fair enough -- unfortunate we can't call this from const context.
>
> OK, so after the dust settled on this discussion, I *think* we ended up
> at this, which I have staged for an upcoming v6. Did I understand you both
> correctly?
>
> commit 1360a440272976df697140361537c5b697d602e0
> Author: John Hubbard <jhubbard@nvidia.com>
> Date:   Thu Feb 19 14:44:02 2026 -0800
>
>     rust: ptr: add const_align_up()
>     
>     Add const_align_up<ALIGN>() to kernel::ptr as the const-compatible
>     equivalent of Alignable::align_up(). This uses inline_const to validate
>     the alignment at compile time with a clear error message.
>     
>     Overflow causes a panic, which becomes a compile-time error in const
>     context. For runtime alignment with fallible overflow handling, callers
>     should use Alignable::align_up() instead.
>     
>     Suggested-by: Danilo Krummrich <dakr@kernel.org>
>     Suggested-by: Miguel Ojeda <ojeda@kernel.org>
>     Signed-off-by: John Hubbard <jhubbard@nvidia.com>
>
> diff --git a/rust/kernel/ptr.rs b/rust/kernel/ptr.rs
> index 5b6a382637fe..b8b0cb1e0b6b 100644
> --- a/rust/kernel/ptr.rs
> +++ b/rust/kernel/ptr.rs
> @@ -225,3 +225,31 @@ fn align_up(self, alignment: Alignment) -> Option<Self> {
>  }
>  
>  impl_alignable_uint!(u8, u16, u32, u64, usize);
> +
> +/// Aligns `value` up to `ALIGN` at compile time.
> +///
> +/// This is the const-compatible equivalent of [`Alignable::align_up`].
> +/// `ALIGN` must be a power of two (enforced at compile time).
> +///
> +/// Panics on overflow, which becomes a compile-time error in const context.
> +/// For runtime alignment with fallible overflow handling, use
> +/// [`Alignable::align_up`] instead.
> +///
> +/// # Examples
> +///
> +/// ```
> +/// use kernel::ptr::const_align_up;
> +/// use kernel::sizes::SZ_4K;
> +///
> +/// assert_eq!(const_align_up::<16>(0x4f), 0x50);
> +/// assert_eq!(const_align_up::<16>(0x40), 0x40);
> +/// assert_eq!(const_align_up::<SZ_4K>(1), SZ_4K);
> +/// ```
> +#[inline(always)]
> +pub const fn const_align_up<const ALIGN: usize>(value: usize) -> usize {
> +    const { assert!(ALIGN.is_power_of_two(), "ALIGN must be a power of two") };
> +    match value.checked_add(ALIGN - 1) {
> +        Some(v) => v & !(ALIGN - 1),
> +        None => panic!("const_align_up: overflow"),
> +    }
> +}

The implementation doesn't address any of my original comment and all my points
still apply.

Best,
Gary

> <blueforge> linux-github (nova-core-blackwell-v6)$ 
>
>
> thanks,


  reply	other threads:[~2026-03-04 11:18 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-21  2:09 [PATCH v5 00/38] gpu: nova-core: firmware: Hopper/Blackwell support John Hubbard
2026-02-21  2:09 ` [PATCH v5 01/38] gpu: nova-core: fix aux device registration for multi-GPU systems John Hubbard
2026-02-24 14:47   ` Danilo Krummrich
2026-02-27 15:37     ` Gary Guo
2026-02-27 15:41       ` Gary Guo
2026-02-27 16:05         ` Danilo Krummrich
2026-02-27 16:29           ` John Hubbard
2026-02-21  2:09 ` [PATCH v5 02/38] gpu: nova-core: pass pdev directly to dev_* logging macros John Hubbard
2026-02-21  2:09 ` [PATCH v5 03/38] gpu: nova-core: print FB sizes, along with ranges John Hubbard
2026-02-21  2:09 ` [PATCH v5 04/38] gpu: nova-core: add FbRange.len() and use it in boot.rs John Hubbard
2026-02-21  2:09 ` [PATCH v5 05/38] gpu: nova-core: Hopper/Blackwell: basic GPU identification John Hubbard
2026-02-21  2:09 ` [PATCH v5 06/38] gpu: nova-core: factor .fwsignature* selection into a new find_gsp_sigs_section() John Hubbard
2026-02-21  2:09 ` [PATCH v5 07/38] gpu: nova-core: use GPU Architecture to simplify HAL selections John Hubbard
2026-02-21  2:09 ` [PATCH v5 08/38] gpu: nova-core: apply the one "use" item per line policy to commands.rs John Hubbard
2026-02-21  2:09 ` [PATCH v5 09/38] gpu: nova-core: move GPU init and DMA mask setup into Gpu::new() John Hubbard
2026-02-21  2:09 ` [PATCH v5 10/38] gpu: nova-core: set DMA mask width based on GPU architecture John Hubbard
2026-02-21  2:09 ` [PATCH v5 11/38] gpu: nova-core: Hopper/Blackwell: skip GFW boot waiting John Hubbard
2026-02-21  2:09 ` [PATCH v5 12/38] gpu: nova-core: move firmware image parsing code to firmware.rs John Hubbard
2026-02-21  2:09 ` [PATCH v5 13/38] gpu: nova-core: factor out an elf_str() function John Hubbard
2026-02-21  2:09 ` [PATCH v5 14/38] gpu: nova-core: don't assume 64-bit firmware images John Hubbard
2026-02-21  2:09 ` [PATCH v5 15/38] gpu: nova-core: add support for 32-bit " John Hubbard
2026-02-21  2:09 ` [PATCH v5 16/38] gpu: nova-core: add auto-detection of 32-bit, 64-bit " John Hubbard
2026-02-21  2:09 ` [PATCH v5 17/38] gpu: nova-core: Hopper/Blackwell: add FMC firmware image, in support of FSP John Hubbard
2026-02-21  2:09 ` [PATCH v5 18/38] gpu: nova-core: Hopper/Blackwell: add FSP falcon engine stub John Hubbard
2026-02-21  2:09 ` [PATCH v5 19/38] gpu: nova-core: Hopper/Blackwell: add FSP falcon EMEM operations John Hubbard
2026-02-21  2:09 ` [PATCH v5 20/38] gpu: nova-core: Hopper/Blackwell: add FSP message infrastructure John Hubbard
2026-02-21  2:09 ` [PATCH v5 21/38] rust: ptr: add const_align_up() and enable inline_const feature John Hubbard
2026-02-21 20:50   ` Miguel Ojeda
2026-02-22 19:03     ` John Hubbard
2026-02-22 19:08       ` Miguel Ojeda
2026-02-23  3:36         ` Alexandre Courbot
2026-02-22  7:46   ` Gary Guo
2026-02-22 19:04     ` John Hubbard
2026-02-23 11:07       ` Danilo Krummrich
2026-02-23 14:16         ` Gary Guo
2026-02-23 14:20           ` Danilo Krummrich
2026-03-04  3:47             ` John Hubbard
2026-03-04 11:18               ` Gary Guo [this message]
2026-03-04 18:53                 ` John Hubbard
2026-03-04 19:04                   ` Gary Guo
2026-03-04 19:14                     ` John Hubbard
2026-03-05  1:23                       ` Alexandre Courbot
2026-03-05  1:31                         ` John Hubbard
2026-03-05  7:07                           ` Alexandre Courbot
2026-03-05 12:28                             ` Gary Guo
2026-03-05 12:36                               ` Danilo Krummrich
2026-03-05 12:59                                 ` Gary Guo
2026-03-05 13:59                               ` Alexandre Courbot
2026-03-05 14:05                                 ` Gary Guo
2026-03-05 15:17                                   ` Alexandre Courbot
2026-02-23 11:23   ` Alice Ryhl
2026-02-21  2:09 ` [PATCH v5 22/38] gpu: nova-core: Hopper/Blackwell: calculate reserved FB heap size John Hubbard
2026-02-21  2:09 ` [PATCH v5 23/38] gpu: nova-core: add MCTP/NVDM protocol types for firmware communication John Hubbard
2026-02-21  2:09 ` [PATCH v5 24/38] gpu: nova-core: Hopper/Blackwell: add FSP secure boot completion waiting John Hubbard
2026-02-21  2:09 ` [PATCH v5 25/38] gpu: nova-core: Hopper/Blackwell: add FSP message structures John Hubbard
2026-02-21  2:09 ` [PATCH v5 26/38] gpu: nova-core: Hopper/Blackwell: add FMC signature extraction John Hubbard
2026-02-21  2:09 ` [PATCH v5 27/38] gpu: nova-core: Hopper/Blackwell: add FSP send/receive messaging John Hubbard
2026-02-21  2:09 ` [PATCH v5 28/38] gpu: nova-core: Hopper/Blackwell: add FspCotVersion type John Hubbard
2026-02-21  2:09 ` [PATCH v5 29/38] gpu: nova-core: Hopper/Blackwell: larger non-WPR heap John Hubbard
2026-02-21  2:09 ` [PATCH v5 30/38] gpu: nova-core: Hopper/Blackwell: add FSP Chain of Trust boot John Hubbard
2026-02-21  2:09 ` [PATCH v5 31/38] gpu: nova-core: Blackwell: use correct sysmem flush registers John Hubbard
2026-02-21  2:09 ` [PATCH v5 32/38] gpu: nova-core: Hopper/Blackwell: larger WPR2 (GSP) heap John Hubbard
2026-02-21  2:09 ` [PATCH v5 33/38] gpu: nova-core: refactor SEC2 booter loading into BooterFirmware::run() John Hubbard
2026-02-21  2:09 ` [PATCH v5 34/38] gpu: nova-core: Hopper/Blackwell: add GSP lockdown release polling John Hubbard
2026-02-21  2:09 ` [PATCH v5 35/38] gpu: nova-core: Hopper/Blackwell: new location for PCI config mirror John Hubbard
2026-02-21  2:09 ` [PATCH v5 36/38] gpu: nova-core: Hopper/Blackwell: integrate FSP boot path into boot() John Hubbard
2026-02-21  2:09 ` [PATCH v5 37/38] rust: sizes: add u64 variants of SZ_* constants John Hubbard
2026-02-21  2:09 ` [PATCH v5 38/38] gpu: nova-core: use SZ_*_U64 constants from kernel::sizes John Hubbard

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=DGTYFUCCECHS.T8EW6ZPL8NZS@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=ecourtney@nvidia.com \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox