Re: [PATCH v12 2/4] mm/slub: allow to set node and align in k[v]realloc

> On Jul 11, 2025, at 5:43 PM, Vlastimil Babka <[email protected]> wrote:
> 
> On 7/11/25 10:58, Harry Yoo wrote:
>> On Wed, Jul 09, 2025 at 07:24:41PM +0200, Vitaly Wool wrote:
>>> Reimplement k[v]realloc_node() to be able to set node and
>>> alignment should a user need to do so. In order to do that while
>>> retaining the maximal backward compatibility, add
>>> k[v]realloc_node_align() functions and redefine the rest of API
>>> using these new ones.
>>> 
>>> While doing that, we also keep the number of  _noprof variants to a
>>> minimum, which implies some changes to the existing users of older
>>> _noprof functions, that basically being bcachefs.
>>> 
>>> With that change we also provide the ability for the Rust part of
>>> the kernel to set node and alignment in its K[v]xxx
>>> [re]allocations.
>>> 
>>> Signed-off-by: Vitaly Wool <[email protected]>
>>> ---
>>> fs/bcachefs/darray.c   |  2 +-
>>> fs/bcachefs/util.h     |  2 +-
>>> include/linux/bpfptr.h |  2 +-
>>> include/linux/slab.h   | 38 +++++++++++++++----------
>>> lib/rhashtable.c       |  4 +--
>>> mm/slub.c              | 64 +++++++++++++++++++++++++++++-------------
>>> 6 files changed, 72 insertions(+), 40 deletions(-)
>> 
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index c4b64821e680..6fad4cdea6c4 100644
>>> --- a/mm/slub.c
>>> +++ b/mm/slub.c
>>> @@ -4845,7 +4845,7 @@ void kfree(const void *object)
>>> EXPORT_SYMBOL(kfree);
>>> 
>>> static __always_inline __realloc_size(2) void *
>>> -__do_krealloc(const void *p, size_t new_size, gfp_t flags)
>>> +__do_krealloc(const void *p, size_t new_size, unsigned long align, gfp_t flags, int nid)
>>> {
>>> void *ret;
>>> size_t ks = 0;
>>> @@ -4859,6 +4859,20 @@ __do_krealloc(const void *p, size_t new_size, gfp_t flags)
>>> if (!kasan_check_byte(p))
>>> return NULL;
>>> 
>>> + /* refuse to proceed if alignment is bigger than what kmalloc() provides */
>>> + if (!IS_ALIGNED((unsigned long)p, align) || new_size < align)
>>> + return NULL;
>> 
>> Hmm but what happens if `p` is aligned to `align`, but the new object is not?
>> 
>> For example, what will happen if we  allocate object with size=64, align=64
>> and then do krealloc with size=96, align=64...
>> 
>> Or am I missing something?
> 
> Good point. We extended the alignment guarantees in commit ad59baa31695
> ("slab, rust: extend kmalloc() alignment guarantees to remove Rust padding")
> for rust in a way that size 96 gives you alignment of 32. It assumes that
> rust side will ask for alignments that are power-of-two and sizes that are
> multiples of alignment. I think if that assumption is still honored than
> this will keep working, but the check added above (is it just a sanity check
> or something the rust side relies on?) doesn't seem correct?
> 

It is a sanity check and it should have looked like this:

        if (!IS_ALIGNED((unsigned long)p, align) && new_size <= ks)
                return NULL;

and the reasoning for this is the following: if we don’t intend to reallocate (new size is not bigger than the original size), but the user requests a larger alignment, it’s a miss. Does that sound reasonable?

~Vitaly