[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZoRz9yEy5wQAu9R9@boqun-archlinux>
Date: Tue, 2 Jul 2024 14:41:11 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Alice Ryhl <aliceryhl@...gle.com>, linux-mm@...ck.org,
David Rientjes <rientjes@...gle.com>,
Christoph Lameter <cl@...ux.com>,
Hyeonggon Yoo <42.hyeyoo@...il.com>,
Roman Gushchin <roman.gushchin@...ux.dev>,
Kees Cook <keescook@...omium.org>, rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH] mm, slab: extend kmalloc() alignment for non
power-of-two sizes
On Tue, Jul 02, 2024 at 11:18:36PM +0200, Vlastimil Babka wrote:
> On 7/2/24 6:40 PM, Alice Ryhl wrote:
> > On Tue, Jul 2, 2024 at 5:58 PM Vlastimil Babka <vbabka@...e.cz> wrote:
> > Thanks!
> >
> > Since this change is motivated by Rust, why not also include the Rust
> > change in this patch? You would need to remove the if inside
> > krealloc_aligned in rust/kernel/alloc/allocator.rs and update the
> > comments.
>
> Right, thanks. Does this look ok? (not tested as I don't have the
> environment working)
>
Thanks! This overall looks good to me.
> diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
> index 229642960cd1..c619acb8b285 100644
> --- a/rust/kernel/alloc/allocator.rs
> +++ b/rust/kernel/alloc/allocator.rs
> @@ -18,23 +18,16 @@ pub(crate) unsafe fn krealloc_aligned(ptr: *mut u8, new_layout: Layout, flags: F
> // Customized layouts from `Layout::from_size_align()` can have size < align, so pad first.
> let layout = new_layout.pad_to_align();
>
> + // Note that `layout.size()` (after padding) is guaranteed to be a multiple of `layout.align()`
> + // which together with the slab guarantee means the `krealloc` will return a properly aligned
> + // object (see comments in `kmalloc()` for more information).
> let mut size = layout.size();
The `mut` needs to be removed since no need to modify `size` afterwards.
>
> - if layout.align() > bindings::ARCH_SLAB_MINALIGN {
> - // The alignment requirement exceeds the slab guarantee, thus try to enlarge the size
> - // to use the "power-of-two" size/alignment guarantee (see comments in `kmalloc()` for
> - // more information).
> - //
> - // Note that `layout.size()` (after padding) is guaranteed to be a multiple of
> - // `layout.align()`, so `next_power_of_two` gives enough alignment guarantee.
> - size = size.next_power_of_two();
> - }
> -
> // SAFETY:
> // - `ptr` is either null or a pointer returned from a previous `k{re}alloc()` by the
> // function safety requirement.
> // - `size` is greater than 0 since it's either a `layout.size()` (which cannot be zero
> - // according to the function safety requirement) or a result from `next_power_of_two()`.
> + // according to the function safety requirement)
The word "either" needs to be removed as well.
With these changes,
Reviewed-by: Boqun Feng <boqun.feng@...il.com>
Regards,
Boqun
> unsafe { bindings::krealloc(ptr as *const core::ffi::c_void, size, flags.0) as *mut u8 }
> }
>
>
Powered by blists - more mailing lists