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 03:49:26PM +1000, Brendan Shephard wrote:
> On Tue, Nov 25, 2025 at 09:23:59AM +0000, Alice Ryhl wrote:
> > On Fri, Nov 21, 2025 at 02:04:28PM +1000, bshephar@bne-home.net wrote:
> > >  impl NovaObject {
> > >      /// Create a new DRM GEM object.
> > >      pub(crate) fn new(dev: &NovaDevice, size: usize) -> Result<ARef<gem::Object<Self>>> {
> > > -        let aligned_size = size.next_multiple_of(1 << 12);
> > > -
> > > -        if size == 0 || size > aligned_size {
> > > +        // Check for 0 size or potential usize overflow before calling page_align
> > > +        if size == 0 || size > usize::MAX - PAGE_SIZE + 1 {
> > 
> > Maybe this should use isize::MAX as the maximum size instead? That's a
> > pretty common maximum size for allocations in Rust and big enough for
> > everyone.
> > 
> > Alice
> 
> Thanks for the review Alice. I used usize here because the page_align()
> function specifically mentions that the provided value should not
> overflow a ['usize'].
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/page.rs#n30
> 
> I don't think that alone needs to be the deciding factor, but it's worth
> expressing why I made that decision to begin with. Happy to hear your
> thoughts, and if you still feel isize::MAX is more appropriate given
> this information.

I know that you picked this particular limit to avoid integer overflow.

I think picking a lower limit makes sense because isize::MAX is a
relatively standard choice for maximum allocation sizes. I think it is a
nice choice because it means you can always compute 2*x for any index or
size x without risk of overflow.

But it's up to the nova folks, not me.

Alice