Re: [PATCH 1/1] drm: nova: Align GEM memory allocation to system page size

[Alice Ryhl <aliceryhl@google.com>]

On Wed, Nov 26, 2025 at 10:22:14PM +0900, Alexandre Courbot wrote:
> On Wed Nov 26, 2025 at 6:54 PM JST, Alice Ryhl wrote:
> > On Wed, Nov 26, 2025 at 09:31:46AM +0900, Alexandre Courbot wrote:
> >> On Tue Nov 25, 2025 at 11:59 PM JST, Alice Ryhl wrote:
> >> > On Tue, Nov 25, 2025 at 3:55 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
> >> >>
> >> >> On Tue Nov 25, 2025 at 11:41 PM JST, Alice Ryhl wrote:
> >> >> > On Tue, Nov 25, 2025 at 3:29 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
> >> >> >>
> >> >> >> On Fri Nov 21, 2025 at 1:04 PM JST, bshephar wrote:
> >> >> >> >              return Err(EINVAL);
> >> >> >> >          }
> >> >> >> >
> >> >> >> > +        let aligned_size = page_align(size);
> >> >> >>
> >> >> >> `page_align` won't panic on overflow, but it will still return an
> >> >> >> invalid size. This is a job for `kernel::ptr::Alignment`, which let's
> >> >> >> you return an error when an overflow occurs.
> >> >> >
> >> >> > The Rust implementation of page_align() is implemented as (addr +
> >> >> > (PAGE_SIZE - 1)) & PAGE_MASK, which definitely will panic on overflow
> >> >> > if the appropriate config options are enabled.
> >> >>
> >> >> That's right, I skimmed its code too fast. ^_^; All the more reason to
> >> >> use `Alignment`.
> >> >
> >> > Alignment stores values that are powers of two, not multiples of PAGE_SIZE.
> >> 
> >> Isn't PAGE_SIZE always a power of two though?
> >
> > Yes it is. Maybe you can elaborate on how you wanted to use Alignment?
> > It sounds like you have something different in mind than what I thought.
> 
> I thought we could just do something like this:
> 
>     use kernel::ptr::{Alignable, Alignment};
> 
>     let aligned_size = size
>         .align_up(Alignment::new::<PAGE_SIZE>())
>         .ok_or(EOVERFLOW)?;
> 
> (maybe we could also have that `Alignment<PAGE_SIZE>` stored as a const
> in `page.rs` for convenience, as it might be used often)

If you're trying to round up a number to the next multiple of PAGE_SIZE,
then you should use page_align() because that's exactly what the
function does.

If you think there is something wrong with the design of page_align(),
change it instead of reimplemtning it.

Alice