On Fri, Aug 1, 2025 at 11:29 AM Danilo Krummrich <d...@kernel.org> wrote: > > On Fri Aug 1, 2025 at 11:18 AM CEST, Alice Ryhl wrote: > > On Thu, Jul 31, 2025 at 05:48:07PM +0200, Danilo Krummrich wrote: > >> drm::Device is allocated through __drm_dev_alloc() (which uses > >> kmalloc()) and the driver private data, <T as drm::Driver>::Data, is > >> initialized in-place. > >> > >> Due to the order of fields in drm::Device > >> > >> pub struct Device<T: drm::Driver> { > >> dev: Opaque<bindings::drm_device>, > >> data: T::Data, > >> } > > > > I'm not convinced this patch is right. > > > > Imagine this scenario: T::Data has size and alignment both equal to 16, > > and lets say that drm_device has a size that is a multiple of 8 but not > > 16 such as 72. In that case, you will allocate 72+16=88 bytes for > > Device, but actually the size of Device is 96 because there is 8 bytes > > of padding between dev and data. > > Are you saying that there is an issue with > > (1) the existing implementation with uses mem::size_of::<Self>() or > > (2) the proper one that uses Kmalloc::aligned_layout(Layout::new::<Self>())? > > I think neither has, because we're not allocating > size_of::<Opaque<bindings::drm_device>>() + size_of::<T::Data>() as you seem > to > assume above, but size_of::<Device<T>>().
Yes, you're right. I misunderstood how __drm_dev_alloc works. Alice