rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Harry Yoo <harry.yoo@oracle.com>, Vitaly Wool <vitaly.wool@konsulko.se>
Cc: linux-mm@kvack.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, Uladzislau Rezki <urezki@gmail.com>,
	Danilo Krummrich <dakr@kernel.org>,
	Alice Ryhl <aliceryhl@google.com>,
	rust-for-linux@vger.kernel.org,
	Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	"Liam R . Howlett" <Liam.Howlett@oracle.com>,
	Kent Overstreet <kent.overstreet@linux.dev>,
	linux-bcachefs@vger.kernel.org, bpf@vger.kernel.org,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Jann Horn <jannh@google.com>, Pedro Falcato <pfalcato@suse.de>
Subject: Re: [PATCH v12 2/4] mm/slub: allow to set node and align in k[v]realloc
Date: Fri, 11 Jul 2025 17:43:16 +0200	[thread overview]
Message-ID: <5bc89531-ab09-4690-aae4-a44f9ddb4a68@suse.cz> (raw)
In-Reply-To: <aHDSLyHZ8b1ELeWe@hyeyoo>

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 <vitaly.wool@konsulko.se>
>> ---
>>  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?

>> +	/*
>> +	 * 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(virt_to_page(p)))
>> +		goto alloc_new;
>> +
>>  	if (is_kfence_address(p)) {
>>  		ks = orig_size = kfence_ksize(p);
>>  	} else {
> 


  parent reply	other threads:[~2025-07-11 15:43 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-09 17:23 [PATCH v12 0/4] support large align and nid in Rust allocators Vitaly Wool
2025-07-09 17:24 ` [PATCH v12 1/4] mm/vmalloc: allow to set node and align in vrealloc Vitaly Wool
2025-07-09 19:01   ` Liam R. Howlett
2025-07-10 14:07     ` Vitaly Wool
     [not found]     ` <D0D76B82-E390-498E-AE84-1B2CA6C0F809@konsulko.se>
2025-07-10 15:09       ` Liam R. Howlett
2025-07-10 15:19       ` Lorenzo Stoakes
2025-07-10 18:57         ` Vitaly Wool
2025-07-09 22:53   ` Alexei Starovoitov
2025-07-09 22:57     ` Danilo Krummrich
2025-07-09 23:14       ` Alexei Starovoitov
2025-07-09 23:26         ` Danilo Krummrich
2025-07-10  0:39           ` Alexei Starovoitov
2025-07-10  6:16             ` Vitaly Wool
2025-07-10  9:57             ` Danilo Krummrich
2025-07-09 17:24 ` [PATCH v12 2/4] mm/slub: allow to set node and align in k[v]realloc Vitaly Wool
2025-07-10  7:45   ` Vlastimil Babka
2025-07-11  8:58   ` Harry Yoo
2025-07-11 11:11     ` Kent Overstreet
2025-07-11 15:43     ` Vlastimil Babka [this message]
2025-07-12 12:43       ` Vitaly Wool
2025-07-14  8:14         ` Vlastimil Babka
2025-07-14 15:27           ` Vitaly Wool
2025-07-15  7:03           ` Alice Ryhl
2025-07-09 17:24 ` [PATCH v12 3/4] rust: add support for NUMA ids in allocations Vitaly Wool
2025-07-09 20:58   ` Danilo Krummrich
2025-07-09 17:25 ` [PATCH v12 4/4] rust: support large alignments " Vitaly Wool
2025-07-09 21:00   ` Danilo Krummrich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5bc89531-ab09-4690-aae4-a44f9ddb4a68@suse.cz \
    --to=vbabka@suse.cz \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=aliceryhl@google.com \
    --cc=bpf@vger.kernel.org \
    --cc=dakr@kernel.org \
    --cc=harry.yoo@oracle.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=jannh@google.com \
    --cc=kent.overstreet@linux.dev \
    --cc=linux-bcachefs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=pfalcato@suse.de \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=urezki@gmail.com \
    --cc=vitaly.wool@konsulko.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).