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 8F79CC3DA7F for ; Thu, 15 Aug 2024 12:29:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0A5A36B00D3; Thu, 15 Aug 2024 08:29:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 0074B6B00D4; Thu, 15 Aug 2024 08:29:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D9B9E6B00D5; Thu, 15 Aug 2024 08:29:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id AFEB86B00D3 for ; Thu, 15 Aug 2024 08:29:42 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 4040DA8E1C for ; Thu, 15 Aug 2024 12:29:42 +0000 (UTC) X-FDA: 82454411004.29.53A598C Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by imf24.hostedemail.com (Postfix) with ESMTP id C9990180030 for ; Thu, 15 Aug 2024 12:29:39 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=j4Rtufy2; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf24.hostedemail.com: domain of dakr@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=dakr@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723724967; a=rsa-sha256; cv=none; b=5Xn6iwPtbghgd6sRD3yYjn4xbpqh61sUQfQvMlGGW9+wHHD+d5CsK05WFMPwKlE5TQ2wCg NwNkQYa1XpBGw1gp7IFEaNNO81vYQDV9wnr7OxN/ld65MoBz2FP1qRfE/87Fj28PjDNei9 SSvm4y2XGelNrud7UOMyaaKkg8YSEfE= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=j4Rtufy2; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf24.hostedemail.com: domain of dakr@kernel.org designates 145.40.73.55 as permitted sender) smtp.mailfrom=dakr@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723724967; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=nn1gyxBacPy0S9OAy3w7UG54Da29PUxMYu/DEPLPCZg=; b=Hz9lvEDUAgRmxZA5xcfVr0ctrolme/DvsWcRycutXZl4bpHdtUmr1Ek2JFwNkT19956JWX 4Q+62+IIul3SzLNJcm4nZfgPVEZjkzsU1ej08+bTw8R5CR+f3DYQIKW14pe7CI8DKNqKzR YNOaa7CT19GEC1FGzC6I6TkAsBLYgTs= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id B9D55CE1C2B; Thu, 15 Aug 2024 12:29:35 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 40B46C32786; Thu, 15 Aug 2024 12:29:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1723724975; bh=01x6idmt70HEhMW6DRE96TN1pZTJ32rMEyQEqY3UF1s=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=j4Rtufy20A/OQeu3YE6pPxD+MmHPkEeVZCXka2ArAUXtG6psKTFS5kbAeCV9zEpll iXIUhQ5wQYuIvf7CasINkIZJO8CzwXvLaPDD2mWj6/ioFLNXiF/cfQjLnQn9vUmUCC kcao4kf7qa4xwxITI/gqDW0eu9s6IFm5/LJQ/J7vDckgwt0mic+AxBf5ZjS1acGE4Z i1ShWNJArYmmjkO+xAX4OFdgfc9jjdy+bBjCjSPMghaccCu57RO/MvuaMG6nnmNYxr HOzhzZ0qxyYxE30yMWda5HlMxXcagtF9efIpgNbDh5LpJ0XWwDLp9Rar3SSIaNpHmy f31yaT7tijLBg== Date: Thu, 15 Aug 2024 14:29:24 +0200 From: Danilo Krummrich To: Benno Lossin Cc: ojeda@kernel.org, alex.gaynor@gmail.com, wedsonaf@gmail.com, boqun.feng@gmail.com, gary@garyguo.net, bjorn3_gh@protonmail.com, a.hindborg@samsung.com, aliceryhl@google.com, akpm@linux-foundation.org, daniel.almeida@collabora.com, faith.ekstrand@collabora.com, boris.brezillon@collabora.com, lina@asahilina.net, mcanal@igalia.com, zhiw@nvidia.com, cjia@nvidia.com, jhubbard@nvidia.com, airlied@redhat.com, ajanulgu@redhat.com, lyude@redhat.com, linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v5 06/26] rust: alloc: implement `Vmalloc` allocator Message-ID: References: <20240812182355.11641-1-dakr@kernel.org> <20240812182355.11641-7-dakr@kernel.org> <5dfe8bae-2c1e-47d4-9fb4-373b7d714c4f@proton.me> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5dfe8bae-2c1e-47d4-9fb4-373b7d714c4f@proton.me> X-Rspam-User: X-Rspamd-Queue-Id: C9990180030 X-Rspamd-Server: rspam01 X-Stat-Signature: 6mgahac5qkas39i8ckondtnk9fe37cid X-HE-Tag: 1723724979-211186 X-HE-Meta: U2FsdGVkX1/mWLFD9CW+Id626ObYOFJdgbgaWwB3utibNxsalDVbOuqq8mgcEzmo8dUAH6N71THmpToO4mMq3g13M5ad/v15FxWIHDYqxH1QzHNUMa0Kf0FqjiQaVmXO1TEVXKuUA4SkLW78IV4mK3puT4+RiJ231rnzun+gOu2GrXkG983iVzIZqMJCbKiC8t/wyYLrySiZ+KYy5OwWHtYz5NwPx+qB3y0EC4AA3BML4fV7MHCYKEtM52vW4HC4OCLDAUgtotZ4nCBpvGPDNwmNZ63H+DAA4rXThkYfq37wuEUs9aJ8hEHquw5pDCABVbSfeDtrZucI69KPs6ynLA+6E843taqg4fmfQNORo+g1k5a+4j9u9n/6cs1DFq0FlrH4XI0uGOCjIKfoqulUIFYHTazpRcJV1SypmUk8XV5gRhrMr/HuEUaiQ+8MgAR004js6PeHOqyJdnm0r4xXxOPCXlQZ4j9LCvfEzCBAP4jyWEPTWiLWrYVVWfROOIGmcJMV87/La4XL5CNkxmhpTIM34bAcR/cpirHSHC0uMcGtWA6CxhDc+I4Tkynl4dGyGouOBDm/J7Y8tXW4c+66aQ2Ogjjl4nEaWMYyE94l9wWCDInNoZIOxo5aTR79Dl9dkSu2Rv1kC5ltJojX6OrKBaeY0NkMsBFqoH6rgEtMFa9sDh9HVXO4K2kH/vEdzERKzwKlLPP2UdPWkg5Ts6KlbVNcVDZtjyeMyTwCylpbnYlW6MzYaEr3exPRcYTlvIb8BGwWz1KiA4hic2BTTbpCuxyxKmtKXf2wFPiEo2BWcHPdvShkRrWyd5npzBdtzS+tOrJCV7DUPvUFNMbBD6muKcvCx/X614e6ERTFgrES84u02MR5He+irTf1F1vmONnXbs6ayNhuDFAYei8LXGR8Bpo2JhqLi3X+l3oVbdlHy+o3Ze9PWp1ZhlPqpG/0V32919NGSg0uB37qbzC6cWO 27Bhx4ca s8WaJxz55hQ7WyQ/YuCRnZs/pyHwSLE0w25VL6oM0FwOKJL4LO1Ite6pomLQ3x3WInFAmUDBtZ0WBwQLqc4oj+u5fgqWuJjgddmyLXH6jClervpYG6TOBcRNfkTV4L1R85+yICGwDy/7BxS7EGoTZ/kG4bRlBBVeD1brcUg9tZ1DvQn9+SR7RrGYX1NERdg84U1kzKld9wvER0OBzPz5gp6/kEK2WVdGkBdYfvs51c1uFKUYAd5W0EIanWZuQ41Qs/t6OuSvAtI0By4lOfqmYQvb38xBkFWc8uduw 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 Thu, Aug 15, 2024 at 06:48:19AM +0000, Benno Lossin wrote: > On 15.08.24 01:20, Danilo Krummrich wrote: > > On Thu, Aug 15, 2024 at 12:13:06AM +0200, Danilo Krummrich wrote: > >> > >>> > >>>> + ptr: Option>, > >>>> + layout: Layout, > >>>> + flags: Flags, > >>>> + ) -> Result, AllocError> { > >>>> + // TODO: Support alignments larger than PAGE_SIZE. > >>>> + if layout.align() > bindings::PAGE_SIZE { > >>>> + pr_warn!("Vmalloc does not support alignments larger than PAGE_SIZE yet.\n"); > >>>> + return Err(AllocError); > >>> > >>> I think here we should first try to use `build_error!`, most often the > >>> alignment will be specified statically, so it should get optimized away. > >> > >> Sure, we can try that first. > > > > I think I spoke too soon here. I don't think `build_error!` or `build_assert!` > > can work here, it would also fail the build when the compiler doesn't know the > > value of the alignment, wouldn't it? I remember that I wasn't overly happy about > > failing this on runtime either when I first thought about this case, but I also > > couldn't think of something better. > > Yes, it might fail even though the alignment at runtime will be fine. > But that's why I suggested trying `build_error!`(or `build_assert!`) > first, if nobody hits the case where the compiler cannot figure it out, > then we can keep it. If there are instances, where it fails, but the > alignment would be fine at runtime, then we can change it to the above. > (I would add such a comment above the assert). Unfortunately, it already does fail with just the test cases. Anyway, even if it would have been fine, I don't think it would have been nice for a future user to run into a build error even though the alignment is perfectlly within bounds. > > > In the end it's rather unlikely to ever hit this case, and probably even more > > unlikely to hit it for a sane reason. > > Yeah, but I still prefer the build to fail, rather than emitting a warn > message that can be overlooked at runtime. > > >>> How difficult will it be to support this? (it is a weird requirement, > >>> but I dislike just returning an error...) > >> > >> It's not difficult to support at all. But it requires a C API taking an > >> alignment argument (same for `KVmalloc`). > > I see, that's good to know. > > >> Coming up with a vrealloc_aligned() is rather trivial. kvrealloc_aligned() would > >> be a bit weird though, because the alignment argument could only be really > >> honored if we run into the vrealloc() case. For the krealloc() case it'd still > >> depend on the bucket size that is selected for the requested size. > > Yeah... Maybe some more logic on the Rust side can help with that. Only if we reimplement `KVmalloc` in Rust, However, there are quite some special cases in __kvmalloc_node_noprof(), i.e. fixup page flags, sanity check the size on kmalloc failure, fail on certain page flags, etc. I don't really want to duplicate this code, unless we absolutely have to. > > >> Adding the C API, I'm also pretty sure someone's gonna ask what we need an > >> alignment larger than PAGE_SIZE for and if we have a real use case for that. > >> I'm not entirely sure we have a reasonable answer for that. > > We could argue that we can remove an "ugly hack" (when we don't have the > build assert, if we do have that, I don't mind not supporting it), but I > agree that finding a user will be difficult. I'd argue it's not really a hack to fail on something that's not supported (yet). Allocations can (almost) always fail, this is just another case. > > >> I got some hacked up patches for that, but I'd rather polish and send them once > >> we actually need it. > > Sure, just wanted to check why you don't want to do it this series. > > --- > Cheers, > Benno >