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 8D280EE4991 for ; Wed, 11 Sep 2024 11:03:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DDB1D940027; Wed, 11 Sep 2024 07:03:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D63E7940021; Wed, 11 Sep 2024 07:03:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C054B940027; Wed, 11 Sep 2024 07:03:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id A0640940021 for ; Wed, 11 Sep 2024 07:03:10 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 13E70415AD for ; Wed, 11 Sep 2024 11:03:10 +0000 (UTC) X-FDA: 82552170540.30.CCCD98A Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf28.hostedemail.com (Postfix) with ESMTP id 4F0C6C000D for ; Wed, 11 Sep 2024 11:03:08 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=H8JOiQgd; spf=pass (imf28.hostedemail.com: domain of dakr@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=dakr@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1726052484; 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=icIb5c4xhrmO7kVLDmWmbI1yxH/qlIG8SiX90yehTTs=; b=HWcsQ0OW3ovJmxIpOng6f5mw64mgUUWBL9MFY4kIvuk9JT3UU8FqGV1txXVzgAeWAB+rc2 SnpQ9AOuXi+HpZHXgOhRXKY331jhCEVBMZq/ya5mS6C0u8MB4iwjJaZN+g1HeRtoIZfzrh 89k8AyvfNjB+QhKKrCp9KAnQaGG2q+Q= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1726052484; a=rsa-sha256; cv=none; b=F7iWCt1qdx/kxOMQzF2pI7TDGSmyT8sX6+jgEACKG2H3yOzE7xkrJujnbPtAMNr6BtwuAE jeGYT6RmA/acx55Vs18deCDwWClkjozC9Tpt/5aH9cf/XLj5zS37/3w7deOS6RzO6qAFX3 fifJoczhi5dn+4nIOG4FuMxG0/ajyGQ= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=H8JOiQgd; spf=pass (imf28.hostedemail.com: domain of dakr@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=dakr@kernel.org; dmarc=pass (policy=quarantine) header.from=kernel.org Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 979D15C0636; Wed, 11 Sep 2024 11:03:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0A9DAC4CEC5; Wed, 11 Sep 2024 11:03:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1726052586; bh=30PyCaGPw86nPfq4/D2gDA2paLlWYzbM2d1hzDaciZY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=H8JOiQgdNrhOJwCp2OCKdb3xqA+Z8hWOwHlps5zaVb2y2DDyVGpvhfVo/BCA7FFzt aX1QcdwxSaMgBeC55TEo7fm00U7cxqxox4Npi+ispeSFMibD/9ECY5tAi5GzTW/q2V Nxh4SioLfHXSYjaytl5r1RkyW+VqAYhaDkbWX/v4T1qu9OudYtsqPbabpqhshduAoq mzOmWrARD73NhUnPkGVoaZk+XwnVR0fIT5/Q+2JA9saxnZ4oZZdrSi4bidV1sCZXjP UFLqYOm9GvWWtDkOLDMrCa+T7aSicaJjOcDVzJwNeXxZA2Viei8Aaj3hQAQSvL2rtO jnioDidiQ4RVw== Date: Wed, 11 Sep 2024 13:02:59 +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 v6 09/26] rust: alloc: implement kernel `Box` Message-ID: References: <20240816001216.26575-1-dakr@kernel.org> <20240816001216.26575-10-dakr@kernel.org> <0146cb78-5a35-4d6b-bc75-fed1fc0c270c@proton.me> <19d81a27-9600-4990-867c-624104e3ad83@proton.me> <77b91448-a21e-4e1b-9a8e-3d2052d79a78@proton.me> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <77b91448-a21e-4e1b-9a8e-3d2052d79a78@proton.me> X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 4F0C6C000D X-Stat-Signature: a7k1efj1kgdyjipq18mmswjz47s7cdwf X-HE-Tag: 1726052588-798888 X-HE-Meta: U2FsdGVkX1/MrXKs8Z2e5wNAZ1bwT+WbWIfo/oyrwyG5xft0xQtn7KgzaxySDVv0F1m53peyBwi/yOEVmGIdxpIaXPWIAK3Z0FeRwVlWZ8A4QAIsm8OLVIlJ/VVS0sQf+zttb9tQXo77S9YRbAncWFRKO5sniV1ngyC1p7g54/9zM+nYDVaUlXLiTT4BK7dJEsxGVOt/+rMiQ2geQdl+Hiz5IDsMUTuDboTSdLnOjglk7k/mKpy9oS51zLcFFnbRwJuvqxiMMDE7Kw3Gi0fdRE4HmCpHVSOO5Z904BeEaItWhvJstuAvQs/ZRPdZA6mOZkjy39QERMN829ue7LzJebLuU2TUpCxWlQMrnSbWV1VwZ8bne5N2+iiO6dsnSPMjaHHctTK4kv9QG6M+iY7+HJ0P3jmVp+vg0i9PZCq3/KMC/74FZJFJ/59C0J32DrHw0dXSddoXF+i9p3c9neNNFAQg5o8KSP6dioDjQ/L/t5Gp/QECchZkxOiIlZ66WxkNDFLgZAJKmrd4ii2Gebg9ZX/+6ytDg3rT1cGSc9K8TB8ZgmBJfd1FAJ2cLUBc30wGQpP32T3iIr31v6zMO28/hqKE/JWZ4jV1Qgv/P3iubTiYRBQHGxksLfBXB1+XzJeEt/4UsDofN6GMiKkJRLN7ynFlxlQyo4AKla+AvY8MM2pG0EV9cauWYa6hKmrtBGGwX3sSM+1M06BipNsEXVpCn6FMsEoWdYNNht7n8uO0k2gP8oOP49mHEEIqsBFDMbh+pXvyx9+FyNrZI5524QFDRdcugC+h+cm6a6aHOiQTG8AA0AvmYwc2OrhGqJCON1gIZOnIlZCxKh6O8cQFO5fz6sxWSgDGlsZZaeLj75MEl+00Va5TElRcAJIsFw4bEm2aI0yk4YSL70ypeHSBtyePzqLqwHA+MPl90ovYUuInkB6UbAUt20wm7f6aZI+WgzRtBUvOj9qprW4ZjQiLVPA LsUbrcZ7 vATA8PXhtS8pp/7aAESYWE5W3BqloVRusqM0WR9bDItb6foRvC26W07gWGyWr2DHle8huQoQh+Gqr4zFcFqgvgFq4mGoNfm8qKlmERd7dfSiRNLlxCK9tL5Rr7eKQd2zqqMCrnIQfrHzVOTLIVlDLcwWSSX0CwZAHemY6PKTTjcRHL+S9ILgimiIQIktwfnys4E4TNaRz80/GtgkufzP0D8zXQFOZ2HIoAnMQqF569isHVlzsu+FCZ5D9s+TH8kHtbdPeg0ACgD2h1divmbPV4pj8pfjamKPslVt+ZZR5atoArnj/E5mNzuq10g1KYqA9mU6Lx4X3ydWrryiWe/xT72XXrjzD1dDlgreFmsnhIIDjQ87lBpDHiPMpypD8bUUeCHSmBaWbaqDeAFtS7ldrgFrHc8tP4SICoQd7CuB7uTzV95TbO1Ctv4qDEAq5zUH6x7gl 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 Wed, Sep 11, 2024 at 08:36:38AM +0000, Benno Lossin wrote: > On 11.09.24 01:25, Danilo Krummrich wrote: > > On Tue, Sep 10, 2024 at 07:49:42PM +0000, Benno Lossin wrote: > >> On 10.09.24 19:40, Danilo Krummrich wrote: > >>> On Sat, Aug 31, 2024 at 05:39:07AM +0000, Benno Lossin wrote: > >>>> On 16.08.24 02:10, Danilo Krummrich wrote: > >>>>> +/// # Examples > >>>>> +/// > >>>>> +/// ``` > >>>>> +/// let b = KBox::::new(24_u64, GFP_KERNEL)?; > >>>>> +/// > >>>>> +/// assert_eq!(*b, 24_u64); > >>>>> +/// # Ok::<(), Error>(()) > >>>>> +/// ``` > >>>>> +/// > >>>>> +/// ``` > >>>>> +/// # use kernel::bindings; > >>>>> +/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1; > >>>>> +/// struct Huge([u8; SIZE]); > >>>>> +/// > >>>>> +/// assert!(KBox::::new_uninit(GFP_KERNEL | __GFP_NOWARN).is_err()); > >>>>> +/// ``` > >>>> > >>>> It would be nice if you could add something like "KBox can't handle big > >>>> allocations:" above this example, so that people aren't confused why > >>>> this example expects an error. > >>> > >>> I don't think that's needed, it's implied by > >>> `SIZE == bindings::KMALLOC_MAX_SIZE + 1`. > >>> > >>> Surely, we could add it nevertheless, but it's not very precise to just say "big > >>> allocations". And I think this isn't the place for lengthy explanations of > >>> `Kmalloc` behavior. > >> > >> Fair point, nevertheless I find examples a bit more useful, when the > >> intention behind them is not only given as code. > >> > >>>>> +/// > >>>>> +/// ``` > >>>>> +/// # use kernel::bindings; > >>>>> +/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1; > >>>>> +/// struct Huge([u8; SIZE]); > >>>>> +/// > >>>>> +/// assert!(KVBox::::new_uninit(GFP_KERNEL).is_ok()); > >>>>> +/// ``` > >>>> > >>>> Similarly, you could then say above this one "Instead use either `VBox` > >>>> or `KVBox`:" > >>>> > >>>>> +/// > >>>>> +/// # Invariants > >>>>> +/// > >>>>> +/// The [`Box`]' pointer is always properly aligned and either points to memory allocated with `A` > >>>> > >>>> Please use `self.0` instead of "[`Box`]'". > >>>> > >>>>> +/// or, for zero-sized types, is a dangling pointer. > >>>> > >>>> Probably "dangling, well aligned pointer.". > >>> > >>> Does this add any value? For ZSTs everything is "well aligned", isn't it? > >> > >> ZSTs can have alignment and then unaligned pointers do exist for them > >> (and dereferencing them is UB!): > > > > Where is this documented? The documentation says: > > > > "For operations of size zero, *every* pointer is valid, including the null > > pointer. The following points are only concerned with non-zero-sized accesses." > > [1] > > That's a good point, the documentation looks a bit outdated. I found > this page in the nomicon: https://doc.rust-lang.org/nomicon/vec/vec-zsts.html > The first iterator implementation has an alignment issue. (Nevertheless, > that chapter of the nomicon is probably useful to you, since it goes > over implementing `Vec`, but maybe you already saw it) > > > [1] https://doc.rust-lang.org/std/ptr/index.html > > Might be a good idea to improve/complain about this at the rust project. Well, my point is how do we know? There's no language specification and the documentation is (at least) ambiguous. > > >> #[repr(align(64))] > >> struct Token; > >> > >> fn main() { > >> let t = 64 as *mut Token; > >> let t = unsafe { t.read() }; // this is fine. > >> let t = 4 as *mut Token; > >> let t = unsafe { t.read() }; // this is UB, see below for miri's output > >> } > >> > >> Miri complains: > >> > >> error: Undefined Behavior: accessing memory based on pointer with alignment 4, but alignment 64 is required > >> --> src/main.rs:8:22 > >> | > >> 8 | let t = unsafe { t.read() }; // this is UB, see below for miri's output > >> | ^^^^^^^^ accessing memory based on pointer with alignment 4, but alignment 64 is required > >> | > >> = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior > >> = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information > >> = note: BACKTRACE: > >> = note: inside `main` at src/main.rs:8:22: 8:30 > > > > `read` explicitly asks for non-null and properly aligned even if `T` has size > > zero. I mentioned this because for `read` it's explicitly documented. However, the nomicon also says "This is possibly needless pedantry because ptr::read is a noop for a ZST, [...]". > > Dereferencing (ie `*t`) also requires that (I just didn't do it, since > then the `Token` must implement `Copy`). Again, how do you know? The documentation isn't clear about it. > > --- > Cheers, > Benno >