[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <dea8645f-9f2c-40f2-bf39-f51c767d07bc@suse.cz>
Date: Fri, 25 Jul 2025 10:47:22 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Vitaly Wool <vitaly.wool@...sulko.se>, linux-mm@...ck.org
Cc: 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,
Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
"Liam R . Howlett" <Liam.Howlett@...cle.com>,
Kent Overstreet <kent.overstreet@...ux.dev>, linux-bcachefs@...r.kernel.org,
bpf@...r.kernel.org, Herbert Xu <herbert@...dor.apana.org.au>,
Jann Horn <jannh@...gle.com>, Pedro Falcato <pfalcato@...e.de>
Subject: Re: [PATCH v13 2/4] mm/slub: allow to set node and align in
k[v]realloc
On 7/15/25 15:58, 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 <vitaly.wool@...sulko.se>
>
> /**
> - * krealloc - reallocate memory. The contents will remain unchanged.
> + * krealloc_node_align - reallocate memory. The contents will remain unchanged.
> * @p: object to reallocate memory for.
> * @new_size: how many bytes of memory are required.
> + * @align: desired alignment.
It should be better noted that only alignments up to those guaranteed by
kmalloc() (with a link to its doc where it's described) are expected and not
arbitrary ones. So we don't give the wrong impression here.
> * @flags: the type of memory to allocate.
> + * @nid: NUMA node or NUMA_NO_NODE
> *
> * If @p is %NULL, krealloc() behaves exactly like kmalloc(). If @new_size
> * is 0 and @p is not a %NULL pointer, the object pointed to is freed.
> @@ -4946,7 +4962,8 @@ __do_krealloc(const void *p, size_t new_size, gfp_t flags)
> *
> * Return: pointer to the allocated memory or %NULL in case of error
> */
> -void *krealloc_noprof(const void *p, size_t new_size, gfp_t flags)
> +void *krealloc_node_align_noprof(const void *p, size_t new_size, unsigned long align,
> + gfp_t flags, int nid)
> {
> void *ret;
>
> @@ -4955,13 +4972,13 @@ void *krealloc_noprof(const void *p, size_t new_size, gfp_t flags)
> return ZERO_SIZE_PTR;
> }
>
> - ret = __do_krealloc(p, new_size, flags);
> + ret = __do_krealloc(p, new_size, align, flags, nid);
> if (ret && kasan_reset_tag(p) != kasan_reset_tag(ret))
> kfree(p);
>
> return ret;
> }
> -EXPORT_SYMBOL(krealloc_noprof);
> +EXPORT_SYMBOL(krealloc_node_align_noprof);
>
> static gfp_t kmalloc_gfp_adjust(gfp_t flags, size_t size)
> {
> @@ -4992,6 +5009,7 @@ static gfp_t kmalloc_gfp_adjust(gfp_t flags, size_t size)
> * failure, fall back to non-contiguous (vmalloc) allocation.
> * @size: size of the request.
> * @b: which set of kmalloc buckets to allocate from.
> + * @align: desired alignment.
I guess here we should say something similar to what I suggested to
krealloc(). The "size >= align" check below doesn't mean the alignment can
be arbitrary. It would be incompatible with how
kvrealloc_node_align_noprof() works anyway.
> * @flags: gfp mask for the allocation - must be compatible (superset) with GFP_KERNEL.
> * @node: numa node to allocate from
> *
> @@ -5004,19 +5022,22 @@ static gfp_t kmalloc_gfp_adjust(gfp_t flags, size_t size)
> *
> * Return: pointer to the allocated memory of %NULL in case of failure
> */
> -void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, b), gfp_t flags, int node)
> +void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, b), unsigned long align,
> + gfp_t flags, int node)
> {
> void *ret;
>
> /*
> * It doesn't really make sense to fallback to vmalloc for sub page
> - * requests
> + * requests and small alignments
> */
> - ret = __do_kmalloc_node(size, PASS_BUCKET_PARAM(b),
> - kmalloc_gfp_adjust(flags, size),
> - node, _RET_IP_);
> - if (ret || size <= PAGE_SIZE)
> - return ret;
> + if (size >= align) {
I think it's similar to the check in __do_krealloc() in v12 and not
particularly useful. We don't need to support align > size for rust anyway?
Does vmalloc even honor that?
It would also be inconsistent that kvmalloc() would give you this
possibility of size<align, but starting with a small size and size>=align
and then kvrealloc() to size<align wouldn't actually work.
> + ret = __do_kmalloc_node(size, PASS_BUCKET_PARAM(b),
> + kmalloc_gfp_adjust(flags, size),
> + node, _RET_IP_);
> + if (ret || size <= PAGE_SIZE)
> + return ret;
> + }
>
> /* non-sleeping allocations are not supported by vmalloc */
> if (!gfpflags_allow_blocking(flags))
> @@ -5034,7 +5055,7 @@ void *__kvmalloc_node_noprof(DECL_BUCKET_PARAMS(size, b), gfp_t flags, int node)
> * about the resulting pointer, and cannot play
> * protection games.
> */
> - return __vmalloc_node_range_noprof(size, 1, VMALLOC_START, VMALLOC_END,
> + return __vmalloc_node_range_noprof(size, align, VMALLOC_START, VMALLOC_END,
> flags, PAGE_KERNEL, VM_ALLOW_HUGE_VMAP,
> node, __builtin_return_address(0));
> }
> @@ -5078,10 +5099,12 @@ void kvfree_sensitive(const void *addr, size_t len)
> EXPORT_SYMBOL(kvfree_sensitive);
>
> /**
> - * kvrealloc - reallocate memory; contents remain unchanged
> + * kvrealloc_node_align - reallocate memory; contents remain unchanged
> * @p: object to reallocate memory for
> * @size: the size to reallocate
> + * @align: desired alignment
Ditto.
> * @flags: the flags for the page level allocator
> + * @nid: NUMA node id
> *
> * If @p is %NULL, kvrealloc() behaves exactly like kvmalloc(). If @size is 0
> * and @p is not a %NULL pointer, the object pointed to is freed.
> @@ -5099,17 +5122,18 @@ EXPORT_SYMBOL(kvfree_sensitive);
> *
> * Return: pointer to the allocated memory or %NULL in case of error
> */
> -void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags)
> +void *kvrealloc_node_align_noprof(const void *p, size_t size, unsigned long align,
> + gfp_t flags, int nid)
> {
> void *n;
>
> if (is_vmalloc_addr(p))
> - return vrealloc_noprof(p, size, flags);
> + return vrealloc_node_align_noprof(p, size, align, flags, nid);
>
> - n = krealloc_noprof(p, size, kmalloc_gfp_adjust(flags, size));
> + n = krealloc_node_align_noprof(p, size, align, kmalloc_gfp_adjust(flags, size), nid);
> if (!n) {
> /* We failed to krealloc(), fall back to kvmalloc(). */
> - n = kvmalloc_noprof(size, flags);
> + n = kvmalloc_node_align_noprof(size, align, flags, nid);
> if (!n)
> return NULL;
>
> @@ -5125,7 +5149,7 @@ void *kvrealloc_noprof(const void *p, size_t size, gfp_t flags)
>
> return n;
> }
> -EXPORT_SYMBOL(kvrealloc_noprof);
> +EXPORT_SYMBOL(kvrealloc_node_align_noprof);
>
> struct detached_freelist {
> struct slab *slab;
Powered by blists - more mailing lists