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 50A48EDE9AB for ; Tue, 10 Sep 2024 19:49:51 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B8E728D00B3; Tue, 10 Sep 2024 15:49:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B3D288D0002; Tue, 10 Sep 2024 15:49:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A2C998D00B3; Tue, 10 Sep 2024 15:49:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 836498D0002 for ; Tue, 10 Sep 2024 15:49:50 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 232BD80A6A for ; Tue, 10 Sep 2024 19:49:50 +0000 (UTC) X-FDA: 82549868940.04.1E6F4BA Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by imf09.hostedemail.com (Postfix) with ESMTP id 3EFE714000C for ; Tue, 10 Sep 2024 19:49:47 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=NwL46EJy; spf=pass (imf09.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.134 as permitted sender) smtp.mailfrom=benno.lossin@proton.me; dmarc=pass (policy=quarantine) header.from=proton.me ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725997685; 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=NBSy/ukMPUFq4hhiS8sTvxHbE6CJ6MpnNMDJG5LgNIA=; b=rSfEGu+k7iBlOvfFo12LpAhTJqyEMO04tS1B5eXFNH8ypQ3wxJowEiVhKcH8rrpYrMxqbD /eyrQDkyfnrv5yCXYd+uYeNgcUxMWkO8/GoDKjDvmSb+qk2OPQtk3CNWduOECrtI+fQDMf vN6q/o0/t1wgo8eoUyFPwG3c2W0QnHI= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725997685; a=rsa-sha256; cv=none; b=kL9T2e3uD1tWJgEqecKYESfGa1JejYIdyAqj8GZLI/ynXGQEiLYsLTQuVN2BxLhfikAz4w b2PW2UxnF9Gl9Xgyv4meDLnhco48COmDFornaJedK3DYC7rXYmafg0phPpdtWXy9bItUVX LakbjjODINx1AIo088S7lqMcDUZXkwI= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=proton.me header.s=protonmail header.b=NwL46EJy; spf=pass (imf09.hostedemail.com: domain of benno.lossin@proton.me designates 185.70.40.134 as permitted sender) smtp.mailfrom=benno.lossin@proton.me; dmarc=pass (policy=quarantine) header.from=proton.me DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=proton.me; s=protonmail; t=1725997786; x=1726256986; bh=NBSy/ukMPUFq4hhiS8sTvxHbE6CJ6MpnNMDJG5LgNIA=; 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=NwL46EJyGnK9RO/wPG0QgZgUapyz8uQhsLe5hp23XfG3o3P70qLY+TGp4Mq5esA/U dvUSlc/rKdk65qH+xS3enhxTXDnv3ns69MSaTd0OyrqJextYDhYCh/4OkBxifnnBtE ankLDKvUMvDxTi4lZdwEPN/zB17Kht3X2DuIl5Mu5Hd8NOn4lQrvcKsgmbNLvImZWq Bxm6/f00Xx7icAqwvJOkCKJ4idzXzr/idICUXgI9AGbzLooDkxXtC26zya5g8AVkhU 4lexie5ZO4dauIRbiTvRBL5gTYCCUWa0SmiWQclh3M2d6fFwtNknBY6gdJdAxplo5R oiDW4hDSCRFCA== Date: Tue, 10 Sep 2024 19:49:42 +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: <19d81a27-9600-4990-867c-624104e3ad83@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> Feedback-ID: 71780778:user:proton X-Pm-Message-ID: 47c1545a9cd7e38378def8282cdab552c864d3e8 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Stat-Signature: nezyprpp5qp6wyt8uc7pg5pean6ys58u X-Rspamd-Queue-Id: 3EFE714000C X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1725997787-605966 X-HE-Meta: U2FsdGVkX1/BgHnzCwxWLPV3LSHM24InJUZgNh3D9GwGe3v3MZDSUyrJxR4klo5m41hhzPCjvtFUIapgC7y7kbAt31sQwW0Drzn92ZDRpD1Hoi0cU1irDiAm69RhKoCqHDTMTqNmOvGJv4hX9XCxRQ/IqPU7WCUo0kZOoaT2Aj1f3Aa0emucNa0kg6WKPVsI2sO5YJGSQEQQ6hkXaBUteZ9aT9QjTx7XcuQNznU/V9QGr5x0OlV2dZRvCOUlf0puo7P1h43wd9K9zvnnFGE2pMJ0WbwT5r4J2EuXcESqQh22uD4W/5JlICSlLHMy2qKYDiFgnHv8pcImrNZeAf0mNCYCvJUURAppfX6hGsb9VNnuibwn/Q0jYzht+ItrcdH4/u/sXLp7hBLzD1N7wVN00xasaGu6DViM9dG7dU7xx+2rmdXyEisCcKPQj+adoDpdBjrOR1lp9Hnlfw1P7DpmRUsmJ5bM3Id6vzvEpxfk6f6GrX5A6gxZSScsFZ6uyeVintwi4/nUPWGelbwOQGRgIZpvTNHR8CWxERqrT6BWBALPyzD6eV4+kGPtvOKVxn1ThsLDoN5pbGCiUZA+tCdeZcrH5+UE0wxppxrKny9dB0bgo7q/vWTRfklWO4iPCHDcxzrQoLHJTp59Ly7XNyFWBO3qFZl4yXN5DyqRaHBC+KyudKuELU9n1vRf043Z+jgXlAzebnth5JnQuQkT4S06o7tVZXV5jrH6cSpIJSbThhZvAickFP3TgZywjUViw7mv3qReSc4dZ0t5/kOFlXPk0UoskK9l4EoClJsH8cjPIUUrSWUfoBOo+DUIqXIv3bv6XjSGhGutwPddCdgUdPHr+oMDemAiVzsClJHBZAm4XdBFjkBbenSDxzJTUugBCvIOdP1q1+NWWEC3KBOBNIaJYpvE7daRb2Jg/2wNMr4ZpJXoEseEBdk+VgRNFapUEPB6VqdyEJV795OlTLSZF+f gjgQySWv cfGb3dm1qM0eL2TLFH+YZtFo1hwUo3tUTxlbcYK9USbHEHUM2aHiUevtPiEf/kFXgDfNbclaiW9OlI0a5DBtL7wWovGMhsZk/JAlsjSw/I4wQvONg5Orqxl0PcRiQ0apVXaSDFL/S4Fkg7EHjfITevI3KiS2Z0O1n1HlD1+CdVDNlFJclnHrKD3wbGULgK4qv3PxWl0PhephoVXRi+HeOlY0zXDbBwyYDqeGNy5ujWgktkJAI8PEv1WJXVLzAmFv1NgFAHlLHKVZ7hEd/0KSkZ23MeiND7mgmsEO5TEKDA8/v53cvadRlHyJTvXDOIvAqRLOlmCO9nFswDoGpeXzR/FdEt4bvjo3t2wXHGsI0+p5Kce+OdxLV2T5L7qcmfcFL+ls5cCfCsUHljj17xKOP7oIwhXNi8dL1Teoa4sOf2wf/VUiFPm4lfw9ZDnj2zZhtf2MnSWEWtr55Q2P0+nJQPfT7AA== 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 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_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. >=20 > I don't think that's needed, it's implied by > `SIZE =3D=3D bindings::KMALLOC_MAX_SIZE + 1`. >=20 > Surely, we could add it nevertheless, but it's not very precise to just s= ay "big > allocations". And I think this isn't the place for lengthy explanations o= f > `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 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.". >=20 > 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!): #[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 align= ment 4, but alignment 64 is required --> src/main.rs:8:22 | 8 | let t =3D unsafe { t.read() }; // this is UB, see below for mir= i's output | ^^^^^^^^ accessing memory based on pointer wit= h alignment 4, but alignment 64 is required | =3D help: this indicates a bug in the program: it performed an invali= d operation, and caused Undefined Behavior =3D help: see https://doc.rust-lang.org/nightly/reference/behavior-co= nsidered-undefined.html for further information =3D note: BACKTRACE: =3D note: inside `main` at src/main.rs:8:22: 8:30 >>> +#[repr(transparent)] >>> +pub struct Box(NonNull, PhantomData); >>> +impl Box >>> +where >>> + T: ?Sized, >>> + A: Allocator, >>> +{ >>> + /// Creates a new `Box` from a raw pointer. >>> + /// >>> + /// # Safety >>> + /// >>> + /// For non-ZSTs, `raw` must point at an allocation allocated with= `A`that is sufficiently >>> + /// aligned for and holds a valid `T`. The caller passes ownership= of the allocation to the >>> + /// `Box`. >> >> You don't say what must happen for ZSTs. >=20 > Because we don't require anything for a ZST, do we? We require a non-null, well aligned pointer (see above). --- Cheers, Benno >>> + #[inline] >>> + pub const unsafe fn from_raw(raw: *mut T) -> Self { >>> + // INVARIANT: Validity of `raw` is guaranteed by the safety pr= econditions of this function. >>> + // SAFETY: By the safety preconditions of this function, `raw`= is not a NULL pointer. >>> + Self(unsafe { NonNull::new_unchecked(raw) }, PhantomData::) >>> + }