Message-Id: <7099B4C7-311F-466D-B12C-BDC52FE34A65@konsulko.se>
Date: Wed, 9 Jul 2025 15:40:17 +0200
From: Vitaly Wool <vitaly.wool@...sulko.se>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: linux-mm@...ck.org,
 akpm@...ux-foundation.org,
 linux-kernel@...r.kernel.org,
 Uladzislau Rezki <urezki@...il.com>,
 Danilo Krummrich <dakr@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>,
 rust-for-linux@...r.kernel.org,
 Hyeonggon Yoo <42.hyeyoo@...il.com>,
 Roman Gushchin <roman.gushchin@...ux.dev>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 "Liam R. Howlett" <Liam.Howlett@...cle.com>
Subject: Re: [PATCH v11 2/4] mm/slub: allow to set node and align in
 k[v]realloc



> On Jul 8, 2025, at 4:03 PM, Vitaly Wool <vitaly.wool@...sulko.se> wrote:
> 
> 
> 
>> On Jul 8, 2025, at 2:52 PM, Vlastimil Babka <vbabka@...e.cz> wrote:
>> 
>> On 7/7/25 18:49, 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.
>>> 
>>> 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 <vitaly.wool@...sulko.se>
>>> ---
>>> include/linux/slab.h | 40 ++++++++++++++++++---------
>>> mm/slub.c            | 64 ++++++++++++++++++++++++++++++--------------
>>> 2 files changed, 71 insertions(+), 33 deletions(-)
>>> 
>>> diff --git a/include/linux/slab.h b/include/linux/slab.h
>>> index d5a8ab98035c..13abcf4ada22 100644
>>> --- a/include/linux/slab.h
>>> +++ b/include/linux/slab.h
>>> @@ -465,9 +465,15 @@ int kmem_cache_shrink(struct kmem_cache *s);
>>> /*
>>> * Common kmalloc functions provided by all allocators
>>> */
>>> -void * __must_check krealloc_noprof(const void *objp, size_t new_size,
>>> -     gfp_t flags) __realloc_size(2);
>>> -#define krealloc(...) alloc_hooks(krealloc_noprof(__VA_ARGS__))
>>> +void * __must_check krealloc_node_align_noprof(const void *objp, size_t new_size,
>>> +        unsigned long align,
>>> +        gfp_t flags, int nid) __realloc_size(2);
>>> +#define krealloc_node_noprof(_p, _s, _f, _n) \
>>> + krealloc_node_align_noprof(_p, _s, 1, _f, _n)
>>> +#define krealloc_noprof(...) krealloc_node_noprof(__VA_ARGS__, NUMA_NO_NODE)
>>> +#define krealloc_node_align(...) alloc_hooks(krealloc_node_align_noprof(__VA_ARGS__))
>>> +#define krealloc_node(...) alloc_hooks(krealloc_node_noprof(__VA_ARGS__))
>>> +#define krealloc(...) alloc_hooks(krealloc_noprof(__VA_ARGS__))
>> 
>> Hm wonder if krealloc() and krealloc_node_align() would be enough. Is
>> krealloc_node() only used between patch 3 and 4?
>> Also perhaps it would be more concise to only have
>> krealloc_node_align_noprof() with alloc_hooks wrappers filling the
>> NUMA_NO_NODE (and 1), so we don't need to #define the _noprof variant of
>> everything. The _noprof callers are rare so they can just always use
>> krealloc_node_align_noprof() directly and also fill in the NUMA_NO_NODE (and 1).
> 
> I don’t think that krealloc_node() is used at all at the moment. I thought I’d define these to be symmetrical to vmalloc() but if you believe these are redundant, I’m all for removing them.
> 
Well, krealloc_noprof() appears to be used by nommu.c and it feels a bit weird to make nommu code deal with NUMA nodes. So unless you feel strongly about it, I would keep krealloc_noprof().

~Vitaly

>> 
>>> void kfree(const void *objp);
>>> void kfree_sensitive(const void *objp);
>>> @@ -1041,18 +1047,23 @@ static inline __alloc_size(1) void *kzalloc_noprof(size_t size, gfp_t flags)
>>> #define kzalloc(...) alloc_hooks(kzalloc_noprof(__VA_ARGS__))
>>> #define kzalloc_node(_size, _flags, _node) kmalloc_node(_size, (_flags)|__GFP_ZERO, _node)
>>> 
>>> -void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, b), gfp_t flags, int node) __alloc_size(1);
>>> -#define kvmalloc_node_noprof(size, flags, node) \
>>> - __kvmalloc_node_noprof(PASS_BUCKET_PARAMS(size, NULL), flags, node)
>>> +void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, b), unsigned long align,
>>> +      gfp_t flags, int node) __alloc_size(1);
>>> +#define kvmalloc_node_align_noprof(_size, _align, _flags, _node) \
>>> + __kvmalloc_node_noprof(PASS_BUCKET_PARAMS(_size, NULL), _align, _flags, _node)
>>> +#define kvmalloc_node_noprof(_size, _flags, _node) \
>>> + kvmalloc_node_align_noprof(_size, 1, _flags, _node)
>>> +#define kvmalloc_node_align(...) \
>>> + alloc_hooks(kvmalloc_node_align_noprof(__VA_ARGS__))
>>> #define kvmalloc_node(...) alloc_hooks(kvmalloc_node_noprof(__VA_ARGS__))
>> 
>> Ditto.
>> 
>>> 
>>> -#define kvmalloc(_size, _flags) kvmalloc_node(_size, _flags, NUMA_NO_NODE)
>>> -#define kvmalloc_noprof(_size, _flags) kvmalloc_node_noprof(_size, _flags, NUMA_NO_NODE)
>>> +#define kvmalloc_noprof(...) kvmalloc_node_noprof(__VA_ARGS__, NUMA_NO_NODE)
>>> +#define kvmalloc(...) alloc_hooks(kvmalloc_noprof(__VA_ARGS__))
>>> #define kvzalloc(_size, _flags) kvmalloc(_size, (_flags)|__GFP_ZERO)
>>> 
>>> -#define kvzalloc_node(_size, _flags, _node) kvmalloc_node(_size, (_flags)|__GFP_ZERO, _node)
>>> +#define kvzalloc_node(_s, _f, _n) kvmalloc_node(_s, (_f)|__GFP_ZERO, _n)
>>> #define kmem_buckets_valloc(_b, _size, _flags) \
>>> - alloc_hooks(__kvmalloc_node_noprof(PASS_BUCKET_PARAMS(_size, _b), _flags, NUMA_NO_NODE))
>>> + alloc_hooks(__kvmalloc_node_noprof(PASS_BUCKET_PARAMS(_size, _b), 1, _flags, NUMA_NO_NODE))
>>> 
>>> static inline __alloc_size(1, 2) void *
>>> kvmalloc_array_node_noprof(size_t n, size_t size, gfp_t flags, int node)
>>> @@ -1068,13 +1079,16 @@ kvmalloc_array_node_noprof(size_t n, size_t size, gfp_t flags, int node)
>>> #define kvmalloc_array_noprof(...) kvmalloc_array_node_noprof(__VA_ARGS__, NUMA_NO_NODE)
>>> #define kvcalloc_node_noprof(_n,_s,_f,_node) kvmalloc_array_node_noprof(_n,_s,(_f)|__GFP_ZERO,_node)
>>> #define kvcalloc_noprof(...) kvcalloc_node_noprof(__VA_ARGS__, NUMA_NO_NODE)
>>> -
>>> #define kvmalloc_array(...) alloc_hooks(kvmalloc_array_noprof(__VA_ARGS__))
>>> #define kvcalloc_node(...) alloc_hooks(kvcalloc_node_noprof(__VA_ARGS__))
>>> #define kvcalloc(...) alloc_hooks(kvcalloc_noprof(__VA_ARGS__))
>>> 
>>> -void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags)
>>> - __realloc_size(2);
>>> +void *kvrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
>>> +   gfp_t flags, int nid) __realloc_size(2);
>>> +#define kvrealloc_node_align(...) kvrealloc_node_align_noprof(__VA_ARGS__)
>>> +#define kvrealloc_node_noprof(_p, _s, _f, _n) kvrealloc_node_align_noprof(_p, _s, 1, _f, _n)
>>> +#define kvrealloc_node(...) alloc_hooks(kvrealloc_node_noprof(__VA_ARGS__))
>>> +#define kvrealloc_noprof(...) kvrealloc_node_noprof(__VA_ARGS__, NUMA_NO_NODE)
>>> #define kvrealloc(...) alloc_hooks(kvrealloc_noprof(__VA_ARGS__))
>> 
>> Ditto.
>> 
>>> extern void kvfree(const void *addr);
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index c4b64821e680..881244c357dd 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;
>>> +
>>> + /*
>>> +  * If reallocation is not necessary (e. g. the new size is less
>>> +  * than the current allocated size), the current allocation will be
>>> +  * preserved unless __GFP_THISNODE is set. In the latter case a new
>>> +  * allocation on the requested node will be attempted.
>>> +  */
>>> + if (unlikely(flags & __GFP_THISNODE) && nid != NUMA_NO_NODE &&
>>> +      nid != page_to_nid(vmalloc_to_page(p)))
>> 
>> We need virt_to_page() here not vmalloc_to_page().
> 
> Indeed, thanks. It is a c’n’p error, we had virt_to_page() in earlier patchsets (i. e. up until v10).
> 
>> 
>>> + goto alloc_new;
>>> +
>>> if (is_kfence_address(p)) {
>>> ks = orig_size = kfence_ksize(p);
>>> } else {

Powered by blists - more mailing lists