From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id C7B72C83F1B for ; Mon, 14 Jul 2025 15:28:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 698F28D0011; Mon, 14 Jul 2025 11:28:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 649BC8D0001; Mon, 14 Jul 2025 11:28:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 538EB8D0011; Mon, 14 Jul 2025 11:28:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 3F6A68D0001 for ; Mon, 14 Jul 2025 11:28:08 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id E6B851D8AA6 for ; Mon, 14 Jul 2025 15:28:07 +0000 (UTC) X-FDA: 83663251014.21.3643B7A Received: from mailrelay-egress16.pub.mailoutpod3-cph3.one.com (mailrelay-egress16.pub.mailoutpod3-cph3.one.com [46.30.212.3]) by imf02.hostedemail.com (Postfix) with ESMTP id 3273780006 for ; Mon, 14 Jul 2025 15:28:04 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=konsulko.se header.s=rsa1 header.b="Z/BES+gk"; dkim=pass header.d=konsulko.se header.s=ed1 header.b=AQxoKkTN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1752506886; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=CxNCFSgystRqkUUeIIO2FJHvWHtIjD7SOGJQsrFHYEE=; b=wONFlOOz1VqpS47nduTGQv/miHDkRm7CP9nHEM/ZSCUd/5eq7CxQIoeHaC0S6OgOuRNqnt 8VyxAfsgBYO+Sjx+ehw6Rs5IT4appHDe//LCrhDhyOHof9hna5Xpombn5pBTklPzrmnXpd oSnBvW1r6xQlfLq5uKibZ2foB5zmcqo= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1752506886; a=rsa-sha256; cv=none; b=5VmFaX8vWPsQJm2K+61ng2VjFwhYoUYFIJWi1jV6ccAEKsJvitZzuXnZBkClgGaYr/+iIh y5Op+TsHhA3bDKFDEXyY6IHszQ8WrwEgxYUdk+1AaoYrRgTSwK4qOX7YuaZr0tZOvkKlXs 8vFY52r9JU6hUxAHMxHwcCM+dmnpHYY= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=konsulko.se header.s=rsa1 header.b="Z/BES+gk"; dkim=pass header.d=konsulko.se header.s=ed1 header.b=AQxoKkTN; dmarc=none; spf=none (imf02.hostedemail.com: domain of vitaly.wool@konsulko.se has no SPF policy when checking 46.30.212.3) smtp.mailfrom=vitaly.wool@konsulko.se DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1752506881; x=1753111681; d=konsulko.se; s=rsa1; h=content-transfer-encoding:content-type:in-reply-to:from:references:cc:to: subject:mime-version:date:message-id:from; bh=CxNCFSgystRqkUUeIIO2FJHvWHtIjD7SOGJQsrFHYEE=; b=Z/BES+gklQqP0Ut67CFd0O7AKDdxn7SzeAeP9sKo9hDxc5vHyaQluD8u4GBmORfa/K3k6FT/QNg5K +5pSyJLd1VjFqcGU04jt6VNqDAAKY3U2gnBYhHJ6CKvmCDjQu4vgMW4qZXRNtX3EmIVFOTqzZwJzXk zeb8JB1bgVleUMUYPfwoZcku/BGt2s7xNJaMnERDpWZcbDs0R3vrVJURffyKZK5uSNMVcOY8Da6QAH aky+odmUqXjx0O/6U3/CTVeIX0gYVFyMxMidO6cmQQq/CB7FbGRVLsUEonhUKu7GDT93zLKlHdMVZO TpEdBDsXq5yhp1IwW7irPS5z6/BDsZA== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; t=1752506881; x=1753111681; d=konsulko.se; s=ed1; h=content-transfer-encoding:content-type:in-reply-to:from:references:cc:to: subject:mime-version:date:message-id:from; bh=CxNCFSgystRqkUUeIIO2FJHvWHtIjD7SOGJQsrFHYEE=; b=AQxoKkTNHwRxzS8AOTETVDzj4k4Jmbk8Yhmp3MgKFL7e8XlwIfsKyDurTcDTsSbvUFVkpLk9Df/aH 3NS4YiLAQ== X-HalOne-ID: 1ed28d5b-60c7-11f0-bb53-e90f2b8e16ca Received: from [192.168.10.245] (host-90-238-19-233.mobileonline.telia.com [90.238.19.233]) by mailrelay2.pub.mailoutpod2-cph3.one.com (Halon) with ESMTPSA id 1ed28d5b-60c7-11f0-bb53-e90f2b8e16ca; Mon, 14 Jul 2025 15:28:00 +0000 (UTC) Message-ID: <45e705ef-e40b-4dd3-a9b9-1a713df5d4e5@konsulko.se> Date: Mon, 14 Jul 2025 17:27:59 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v12 2/4] mm/slub: allow to set node and align in k[v]realloc To: Vlastimil Babka Cc: Harry Yoo , linux-mm@kvack.org, akpm@linux-foundation.org, linux-kernel@vger.kernel.org, Uladzislau Rezki , Danilo Krummrich , Alice Ryhl , rust-for-linux@vger.kernel.org, Lorenzo Stoakes , "Liam R . Howlett" , Kent Overstreet , linux-bcachefs@vger.kernel.org, bpf@vger.kernel.org, Herbert Xu , Jann Horn , Pedro Falcato References: <20250709172345.1031907-1-vitaly.wool@konsulko.se> <20250709172441.1032006-1-vitaly.wool@konsulko.se> <5bc89531-ab09-4690-aae4-a44f9ddb4a68@suse.cz> <3AD3F7B5-679F-4DC8-968F-9FE991B56A5C@konsulko.se> <1dedcee0-c5a2-47b3-ae13-315ad437ae1a@suse.cz> Content-Language: en-US From: Vitaly Wool In-Reply-To: <1dedcee0-c5a2-47b3-ae13-315ad437ae1a@suse.cz> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Stat-Signature: oyfi5qk6ysfj1ubwe9ky1i3yrkggfrco X-Rspamd-Queue-Id: 3273780006 X-Rspamd-Server: rspam10 X-Rspam-User: X-HE-Tag: 1752506884-179829 X-HE-Meta: U2FsdGVkX1+41Zjp+YTZBS4Ualnj9/r0dtfq3Zsp3cZNd0NpPU152Mx7Gbx4wGWuIEf7JfMIWwgQK3wDiGZT3S8RXysu7uAhOo9VRY2rT1oLjuXlKdYdI6jQcNUsmi35jK3NNHlOOtwzYOgiYnW9UNdFcybmifRqv/o2hsDe3m2W50NT8pQ7x74XVR+FOlw2SpZH5hnhw6BmVTD5sr+H5L8c+kcHq6j74dlHfROOzTy2/zIisJwEbSt2weacpQkI7itMBkqfZ95RCpTdWF2nBKc7nm3MdmJA1vtn3k5vLVAXCubEyLz1WxMrSRAQxLDlmyHQBKx8UTiYa1GfLmulmHDgOfUNsEizwF6jU6ZZ2aN9L48S72GczaABHZnfh6lRCeYJCnfYpJpO4X2cFR9lBGTT1nFyvV1qoxwXAKfMk8dlYM7mqMvCqfHKK/ig7sauxOkmLsfwu698Mytb4n73Az0RdRiyjpHefI5eh+45C3OQ4gMRFul9hjf7e0q9dz+rfP5Fht2tRq07MsFefTnQR8HLhh5VnJzO2jqG+2wR9HZX13DlNO4R2PTfrNCOrfl/+4nv99mxidlYZUXOkqUiezcXZukZtV5tgU5+/o8yUAx88GV6jsBcW+fY3XRO+7I5W6f04QdzYTTfB+xjxA/GQBkqu390wZlMdezW2l+8QUWxt8Tf4Ij4EQeZ7e3raycDmm5lHg1D5Jv8N0VlzhvmXH4DxMKqiF4lGRW4XrKeMo0G/zB43Xkhvq8ylkLLJjMD74isPYxQtguRWC4TnJyG9yCCrPG9WMlXxHP44KfmyfXbbGamvqb7RmNXGphCQgxXEOqOZ6b2v0rOHv3uIK3VTF/gZuTmgG2frZ0c8pM1K9h29bMeoP+AAe9Rzao+mquI60U9C2wmlt2E8S5wVtPE6xqdDXPwzsByX7hGkSiw27u0IHFcy7c7WCE/C7o7TSYMXfGztSYBBEkFA9fJLkB sF5KY/oo L0NfEPcwYLZ41KAMZZm+WhPlicGdcnmubcHqzFYT0bspZPZ1zWST6v/o9wFlGT38SvhAqVuWAjhkxejUTaPtOZYqJr9JKw0mPpyx4pndbDio2uveQAz/iI+tnwAJAimwLMoPhvzwyFdJtG8DlKnm+OzW9+ZFlkEBk/ffClyU8ffo7NToCpS4KhNs5uZS/cfBZA5XV0rTK2ICq3Yse5jjrOrG59Q== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On 7/14/25 10:14, Vlastimil Babka wrote: > On 7/12/25 14:43, Vitaly Wool wrote: >> >> >>> On Jul 11, 2025, at 5:43 PM, Vlastimil Babka wrote: >>> >>> On 7/11/25 10:58, Harry Yoo wrote: >>>> On Wed, Jul 09, 2025 at 07:24:41PM +0200, Vitaly Wool wrote: >>>>> 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? > > So taking a step back indeed the align passed to krealloc is indeed used > only for this check. If it's really just a sanity check, then I'd rather not > add this parameter to krealloc functions at all - kmalloc() itself also > doesn't have it, so it's inconsistent that krealloc() would have it - but > only to return NULL and not e.g. try to reallocate for alignment. > > If it's not just a sanity check, it means it's expected that for some > sequence of valid kvrealloc_node_align() calls it can return NULL and then > rely on the fallback to vmalloc. That would be rather wasteful for the cases > like going from 64 to 96 bytes etc. So in that case it would be better if > krealloc did the reallocation, same as in cases when size increases. Of > course it would still have to rely on the documented alignment guarantees > only and not provide anything arbitrary. aligned_size() in > rust/kernel/alloc/allocator.rs is responsible for that, AFAIK. > > And I think it's not a sanity check but the latter - if the following is a > valid k(v)realloc sequence (from Rust POV). The individual size+align > combinations AFAIK are, but if it's valid to make them follow one another > them like this, I don't know. > > krealloc(size=96, align=32) -> can give object with 32 alignment only > krealloc(size=64, align=64) -> doesn't increase size but wants alignment 64 > We should be able to correctly process these. I agree that making such cases fall back to vrealloc is suboptimal but it's a technically correct behavior. I understand that you would rather have a reallocation on the slub side in these cases, so this will look as if (!IS_ALIGNED((unsigned long)p, align) && new_size <= ks) goto alloc_new; I'll modify/retest for the next patchset iteration. ~Vitaly