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 0914EEE0219 for ; Wed, 11 Sep 2024 08:36:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3F8DB8D00FF; Wed, 11 Sep 2024 04:36:51 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3A8638D0056; Wed, 11 Sep 2024 04:36:51 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 298678D00FF; Wed, 11 Sep 2024 04:36:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 0C8468D0056 for ; Wed, 11 Sep 2024 04:36:51 -0400 (EDT) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 86BCB1214C4 for ; Wed, 11 Sep 2024 08:36:50 +0000 (UTC) X-FDA: 82551801780.29.1864BFF Received: from mail-4316.protonmail.ch (mail-4316.protonmail.ch [185.70.43.16]) by imf06.hostedemail.com (Postfix) with ESMTP id 84F5C180012 for ; Wed, 11 Sep 2024 08:36:48 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=proton.me header.s=q5xqu33p5zemxkr2vym5jgtsx4.protonmail header.b=IGyYJK1x; dmarc=pass (policy=quarantine) header.from=proton.me; spf=pass (imf06.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.43.16 as permitted sender) smtp.mailfrom=benno.lossin@proton.me ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1726043732; a=rsa-sha256; cv=none; b=ZTn6eqMrBXmnlBX+aZXIZIUoUUjXK3758xhoE1zrCITyuSIvySRFySpA6SUYYVFnNywTI3 DjF2+hpSv5AdZDA2ExtlQTOAtUtSgye4h6WeIPMbDHbuK2BmG3EDWKjPS619dXt5ZFdzLl laMfeg69R+oy/v8Hioj5hAYtE+yA2fQ= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=proton.me header.s=q5xqu33p5zemxkr2vym5jgtsx4.protonmail header.b=IGyYJK1x; dmarc=pass (policy=quarantine) header.from=proton.me; spf=pass (imf06.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.43.16 as permitted sender) smtp.mailfrom=benno.lossin@proton.me ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1726043732; 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=KnWn/8TtLlH9g5YMgEkjKP7rMW4YOeNrjWYIMxEWl2U=; b=CBRmPA09/f9SAl5MjgT2PHIcqLi00jNR+v6PwlWYX+zfUHsVDJXx8qMkboken6rBTcCzqN 1eq2sL3dG4ViixsiEMMNa32LQtZKp7Ll50p4L5Rowq7Tv1eBCXkX72rXlNJYFkMDv8oX+q 4+bWdOenYP9U4iTEoLVO7ko45hFYVsM= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=q5xqu33p5zemxkr2vym5jgtsx4.protonmail; t=1726043805; x=1726303005; bh=KnWn/8TtLlH9g5YMgEkjKP7rMW4YOeNrjWYIMxEWl2U=; h=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References: Feedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID: Message-ID:BIMI-Selector; b=IGyYJK1xJG1ico72kpnXAj/gt7H6UCJc5lWmqzFpNeh1O27PFoydWGigCtS85MI3S g6b1zVwWFM8F5sMh4/GK2+Yh85kFDD/LiwizU3w9ypxO0gwiRthE+w5uYG6FQSVBTj kJg+3bf0sH1iFB5RhP6VmgusnwyNrvsIOyl5pwvuwLUKZ5jchcwL12XJN8ef3iAYkM CJYfOyP7DisLRjYtGc6sFq7ECkEB9uM/UhwqahyrIHLTlI4j+gITA4S3J4rIPnzXcj nx5NsBmwizdxQOkYlV0WhG3kNbUxwFZ3q/BA7TdiHK72J8gH3RiKZZgZOmxydeAq+j bq1gJxmaMtchg== Date: Wed, 11 Sep 2024 08:36:38 +0000 To: Danilo Krummrich From: 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: <77b91448-a21e-4e1b-9a8e-3d2052d79a78@proton.me> In-Reply-To: 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> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: 88c99c18904d989862df81bed222d177747790f8 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 84F5C180012 X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: 1j6szbqdqr9opi791138q4ostp8845yz X-HE-Tag: 1726043808-950243 X-HE-Meta: U2FsdGVkX18BQLW9fvaTe/9kWdJ13b5LGaM5BS1aYTfFcIDjthCsiZHObnP9UTc/tDq8bD9L2H4C6JgU5jth4K3dqkxNBj9qngUofh+BjyDnFoVhSqGOBT80zd1hLIk8CvQkvGg0KjsX8/74W/3uha32v6hVZwizWz1TdQ9COaSG4d69TCPE18sy9ZzdinGk2hQS0zvs8cmBvH8IzvdcGWFDe/c/9snHQjViDXEt4RHI/Ue5q/cLwgT9/SRrgepiAFQFRqSUkDG8RH/5ksfCM9vrpBUHzzYug30tiBddS23hjl2kOMG2ObUEVi1F0Yue85Gj7RDHZ1xhm3ssivYQ7SqLMIGNhIyRQmETT2Qv8slpXCObG0h1p28cEtZTz0c1g/cBie5hav9XmxfjUp+Iz2ernSJvUQlkR4HP8uTltsvck1RAXF3yCorJnLCD9TbVyUoDaJ6HUJwzME3zDnZ9z91rjdALJ/skM0gmiD9pey7yRctQ6EQFwrHaMEjECQDZH5728yJYO7OvFH39UpMObGc5fQ5nbVowTqlGx+v67wxHyMBa5Ju8Vc0CNcei3SNdvLt0fXujqv7jsogUlw+AxBS+zJ54JX136hJuWLGWLiuZzSXc0dM3sEE16ZD1z+FdKwmQIp9CfwLAHBw+/lLyHdaJTh2dr5LGizH9unn0OzjZaD4Mo8H7GB4l1B0BswbM79orh+4PmoiLKk6Iu2VAIe+KzhiD42aumy6eb15NR1TOLX9GX2t9CGrlAmhKIOlnLQjRYEKS1RMhk8+YG/6bf7u8Gz5NY93rhixLekxb0fiUNu4ZTpT3/AUp7uGSqxQm7O6UFQEcC+EdF02kzKSIOfUueH+OvSMjUHc7ZHnttVETXxfOVEMFmSvtjqN3MVDAurE67py/8CJTX8VpsImhltl/PcFLgKGYYO6+Bp/kNUYwj0C2yw4oZZxqTL9iIrgkT79EbpD5yH0xpZky+Vc vGsSqVyg ETBxX+ypLg1yekx4kIxsnIRIceuWNQb+BVOAFNPMVgmATOykLagJ4TPTN3gOk39NJSYFchqZPMuNHKHSKqfqxEhGZ/WqNEcCFCs5RoNENMHWpihdpgkjn05oQDgGJZpZ+O5G34boHUtg+ur/QylR9QlawEQTrC91MNrBfzageSIpUZC1RrbMPcZMpMbwiYaVEOvBpLnQI7aqlLZF9+8uqr+fH33dvw5juKkfdzYleBK+ZJ1eCkwiD3TTO029072obuvbel5Q+yQDX1e8rvFhsjr56QPTtTL7YffpJb4KXsjHyOPwwYVkxu+i8laJdZZV6lahYAbQ3t2ETNhHyqWz2dQ+nj6CBnsNxSWxzRBRYpV4+KhkkpOoupiLnh1cjFXDk5R6Io+QSt6haIiz1liIOruDOZbZTNfeK7SunUQDSeeMTqfTmFrv4jhzHqJvzpEgVK4zWANQw8GbEM3HvohDOz7DDggXC+hc6nY6syfSwiZf+LDGVaFePx76Ek9GkicVtyOlvb9RzvFdlIP+NLDgItu1YEg== 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 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 =3D KBox::::new(24_u64, GFP_KERNEL)?; >>>>> +/// >>>>> +/// assert_eq!(*b, 24_u64); >>>>> +/// # Ok::<(), Error>(()) >>>>> +/// ``` >>>>> +/// >>>>> +/// ``` >>>>> +/// # use kernel::bindings; >>>>> +/// const SIZE: usize =3D bindings::KMALLOC_MAX_SIZE as usize + 1; >>>>> +/// struct Huge([u8; SIZE]); >>>>> +/// >>>>> +/// assert!(KBox::::new_uninit(GFP_KERNEL | __GFP_NOWARN).is_e= rr()); >>>>> +/// ``` >>>> >>>> It would be nice if you could add something like "KBox can't handle bi= g >>>> 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 =3D=3D 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 =3D 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 point= s 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 i= t? >> >> ZSTs can have alignment and then unaligned pointers do exist for them >> (and dereferencing them is UB!): >=20 > Where is this documented? The documentation says: >=20 > "For operations of size zero, *every* pointer is valid, including the nul= l > pointer. The following points are only concerned with non-zero-sized acce= sses." > [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.ht= ml 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. >> #[repr(align(64))] >> struct Token; >> >> fn main() { >> let t =3D 64 as *mut Token; >> let t =3D unsafe { t.read() }; // this is fine. >> let t =3D 4 as *mut Token; >> let t =3D unsafe { t.read() }; // this is UB, see below for miri= 's output >> } >> >> Miri complains: >> >> error: Undefined Behavior: accessing memory based on pointer with al= ignment 4, but alignment 64 is required >> --> src/main.rs:8:22 >> | >> 8 | let t =3D 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 >> | >> =3D help: this indicates a bug in the program: it performed an inv= alid operation, and caused Undefined Behavior >> =3D help: see https://doc.rust-lang.org/nightly/reference/behavior= -considered-undefined.html for further information >> =3D note: BACKTRACE: >> =3D note: inside `main` at src/main.rs:8:22: 8:30 >=20 > `read` explicitly asks for non-null and properly aligned even if `T` has = size > zero. Dereferencing (ie `*t`) also requires that (I just didn't do it, since then the `Token` must implement `Copy`). --- Cheers, Benno