Alice Ryhl
2025-Nov-26 09:53 UTC
[PATCH 1/1] drm: nova: Align GEM memory allocation to system page size
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 at 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