From nobody Thu Dec 5 02:13:45 2024 Received: from mail-vs1-f47.google.com (mail-vs1-f47.google.com [209.85.217.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C97B7207A33; Tue, 3 Dec 2024 22:44:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.217.47 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733265845; cv=none; b=ReH2//q3+WYfbuIQBdC3oiokqoKQjnec4ar3/Gn4J3AEmFOwjLfr7qZufbne95K1NMCdI/FAWMUat1EpM/rItKZ4ux52D5h5iED6Z7ENFpZx/qe9nnqZylDp0h1KISm2UFAcifTT96NIUBdxbPz6t3KM82mvP0eu8cb/IKLBdJI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733265845; c=relaxed/simple; bh=vcA0/cRUJDq8fEnBd8BZ5ptOA4cTepV39kADJgFaWhU=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=WglvOsxGJA5utVZjQRpoNVoxp/M6c+1rxPggPbd4nz0xDbWm64zwpjRlQKUfQ+o+VYWb90IxULfLA3HOEQjbKY4tkBruOdp9xcTfv9L0mXQ8gRuWQvjFVuR5wXbX6dSfd0XaPwSrmFJ+9l93nOKCoK1UKkrRHlRTF4PFAPm0EU8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=BDLelqTy; arc=none smtp.client-ip=209.85.217.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="BDLelqTy" Received: by mail-vs1-f47.google.com with SMTP id ada2fe7eead31-4addd900de1so1628690137.3; Tue, 03 Dec 2024 14:44:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1733265842; x=1733870642; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=M8esf/7QZXouMr7p2ENilxF+QnuMFeyQdGO61yRTVrw=; b=BDLelqTy9Ap/0C6Ba0/nYWYPTi9dy/7o9EGVtXqSbjDPb50+USM0cwRKQzKLi/XmiN 0779dV9SgIhXRkVpq8mOfwjHWAEM3w6wengpLoQLcIdsrdtI1bTrgG7XpgYO75NngIao gI2O8mXXKqedjYr6f/w2WJo+P+RId5TcrJl7O4WLYcsBMmbW5nx6N52A4u6pNPxK3UA3 w6x2do7Y+38VPt46sb9X4yGvLy4JSA7/H1eUQdEomTwQ3yqFXN9MPAByoMmrV/GzygwK WPDbA5X5IuNH53GsjPesE01hEoKJazQG34sxHQquMV+LyxAsTG38xGl5QfujD8tXwmnw 3xTg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733265842; x=1733870642; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=M8esf/7QZXouMr7p2ENilxF+QnuMFeyQdGO61yRTVrw=; b=sba/x0H8zl1F+RYWvTmMgqDn7anJlOUWHFiTBbbVNhU1ryo5nrrPI509+ChPpS8pBV QPPdRQXMj8LWAwIryad4xrzUXm/k0Kss1sxVQy+FmywA8Gch36yD0NAGT5toBYUu8Udi UXpMzwdqdhdV5UzE+e7gX78xYQ0gIdTjqkGW8jMKNrs3fleDlAD0AYZsvalRZhg9Ec5E 3Rf5UBcdQJBqTZGxNMYdcS/ltWBb83bXeXvIylQEaEUZ7Nsj9zx8+dqDdDU3GEpuOzzd u3CwPwVPIBm7nuXloBZaPMG2ZEcEUz200RBXhlrl80YU6sBzDovMAkUFfdwvBeJbw7ex p6AQ== X-Forwarded-Encrypted: i=1; AJvYcCUj8Hsojpd80YNGDGA9U9ZSi47CVcMr49p0cKsU+r2OvupzbiKsh9Sy3srIp2sQomimCSmIrHApqr3n6pE=@vger.kernel.org, AJvYcCWDj29J9mqehRK70hEqMycBlhMSLVrt6I9RaNdfVAGI9su/NYF0Ahp3HGAceXfNlX41i9aCp1XGYRCap5JE4mY=@vger.kernel.org X-Gm-Message-State: AOJu0Yz+uXxsQuPAGdk5n65m8uXVBL0DaCECQvarYoCvO4KDf1yaf/4i MjdfzayM+/guwa8HJKwfBWsYqbh4h7KCEHq1QQrfxDjDFPQADl4+ X-Gm-Gg: ASbGncv/eNFiQHAEa6YnEvxh9c52vpIyxOYdyU4nWJO6wrJkmRMDvbYwgI3OK3hfbya xmSL/3gfuK4O7iTqR1dMVQyDd37aNE2FXwIEbNgyXEz2q3KC2P/6yGPXO7Mz6TpgFK7CkiiqSg6 pM/6s1x99UfoCsERk29L5KqW72F+xWHU0gRbhaaqu6Leqz0MVO80Po8K/Sl9+IJcpdwf3U/d0Bs Er4RHntPsreAVYcuySJN50YjA6oFR15qh+qotkl81w1B1ga6fcrDB3skFcYndtPA0DnQ+jKUoqG 9pIMZRHqBYZ4MWYUwq9lcbgSuEKIeWaibC/qQALTNb6qzDNW1PxTPA== X-Google-Smtp-Source: AGHT+IHGfdBGFFuPs1uTYUtQudU2rGCO7znsYtqa3yDm1nuL79AKUFOz09/JaceDMPaqrgQrPxQKuQ== X-Received: by 2002:a05:6102:2ac2:b0:4af:4101:fd53 with SMTP id ada2fe7eead31-4af9735e090mr6623441137.22.1733265842490; Tue, 03 Dec 2024 14:44:02 -0800 (PST) Received: from 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa ([2620:10d:c091:600::1:352d]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7b6849495d2sm555516185a.68.2024.12.03.14.44.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 14:44:02 -0800 (PST) From: Tamir Duberstein Date: Tue, 03 Dec 2024 17:43:50 -0500 Subject: [PATCH v11 1/2] rust: types: add `ForeignOwnable::PointedTo` Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20241203-rust-xarray-bindings-v11-1-58a95d137ec2@gmail.com> References: <20241203-rust-xarray-bindings-v11-0-58a95d137ec2@gmail.com> In-Reply-To: <20241203-rust-xarray-bindings-v11-0-58a95d137ec2@gmail.com> To: Danilo Krummrich , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Alice Ryhl , Trevor Gross Cc: =?utf-8?q?Ma=C3=ADra_Canal?= , Asahi Lina , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Tamir Duberstein X-Mailer: b4 0.15-dev Allow implementors to specify the foreign pointer type; this exposes information about the pointed-to type such as its alignment. This requires the trait to be `unsafe` since it is now possible for implementors to break soundness by returning a misaligned pointer. Encoding the pointer type in the trait (and avoiding pointer casts) allows the compiler to check that implementors return the correct pointer type. This is preferable to directly encoding the alignment in the trait using a constant as the compiler would be unable to check it. Reviewed-by: Andreas Hindborg Signed-off-by: Tamir Duberstein --- rust/kernel/alloc/kbox.rs | 38 ++++++++++++++++++++------------------ rust/kernel/miscdevice.rs | 6 +++--- rust/kernel/sync/arc.rs | 21 ++++++++++++--------- rust/kernel/types.rs | 46 +++++++++++++++++++++++++++++++------------= --- 4 files changed, 66 insertions(+), 45 deletions(-) diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs index 4ffc4e1b22b2b7c2ea8e8ed5b7f7a8534625249f..4e7a0ce9cc9c24f2e828f41e910= 5acc4048333d5 100644 --- a/rust/kernel/alloc/kbox.rs +++ b/rust/kernel/alloc/kbox.rs @@ -349,68 +349,70 @@ fn try_init(init: impl Init, flags: Flags) -= > Result } } =20 -impl ForeignOwnable for Box +// SAFETY: The `into_foreign` function returns a pointer that is well-alig= ned. +unsafe impl ForeignOwnable for Box where A: Allocator, { + type PointedTo =3D T; type Borrowed<'a> =3D &'a T; type BorrowedMut<'a> =3D &'a mut T; =20 - fn into_foreign(self) -> *mut crate::ffi::c_void { - Box::into_raw(self).cast() + fn into_foreign(self) -> *mut Self::PointedTo { + Box::into_raw(self) } =20 - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { // SAFETY: The safety requirements of this function ensure that `p= tr` comes from a previous // call to `Self::into_foreign`. - unsafe { Box::from_raw(ptr.cast()) } + unsafe { Box::from_raw(ptr) } } =20 - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> &'a T { + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> &'a T { // SAFETY: The safety requirements of this method ensure that the = object remains alive and // immutable for the duration of 'a. - unsafe { &*ptr.cast() } + unsafe { &*ptr } } =20 - unsafe fn borrow_mut<'a>(ptr: *mut core::ffi::c_void) -> &'a mut T { - let ptr =3D ptr.cast(); + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> &'a mut T { // SAFETY: The safety requirements of this method ensure that the = pointer is valid and that // nothing else will access the value for the duration of 'a. unsafe { &mut *ptr } } } =20 -impl ForeignOwnable for Pin> +// SAFETY: The `into_foreign` function returns a pointer that is well-alig= ned. +unsafe impl ForeignOwnable for Pin> where A: Allocator, { + type PointedTo =3D T; type Borrowed<'a> =3D Pin<&'a T>; type BorrowedMut<'a> =3D Pin<&'a mut T>; =20 - fn into_foreign(self) -> *mut crate::ffi::c_void { + fn into_foreign(self) -> *mut Self::PointedTo { // SAFETY: We are still treating the box as pinned. - Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }).cast() + Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }) } =20 - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { // SAFETY: The safety requirements of this function ensure that `p= tr` comes from a previous // call to `Self::into_foreign`. - unsafe { Pin::new_unchecked(Box::from_raw(ptr.cast())) } + unsafe { Pin::new_unchecked(Box::from_raw(ptr)) } } =20 - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a T> { + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a T> { // SAFETY: The safety requirements for this function ensure that t= he object is still alive, // so it is safe to dereference the raw pointer. // The safety requirements of `from_foreign` also ensure that the = object remains alive for // the lifetime of the returned value. - let r =3D unsafe { &*ptr.cast() }; + let r =3D unsafe { &*ptr }; =20 // SAFETY: This pointer originates from a `Pin>`. unsafe { Pin::new_unchecked(r) } } =20 - unsafe fn borrow_mut<'a>(ptr: *mut core::ffi::c_void) -> Pin<&'a mut T= > { - let ptr =3D ptr.cast(); + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a mut T> { // SAFETY: The safety requirements for this function ensure that t= he object is still alive, // so it is safe to dereference the raw pointer. // The safety requirements of `from_foreign` also ensure that the = object remains alive for diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index e58807ad28dc644fa384e9c1fb41fd6e53abea7a..ac6b591a83ad558f12ce2746b54= e7f76d8b99c6f 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -193,7 +193,7 @@ impl VtableHelper { }; =20 // SAFETY: The open call of a file owns the private data. - unsafe { (*file).private_data =3D ptr.into_foreign() }; + unsafe { (*file).private_data =3D ptr.into_foreign().cast() }; =20 0 } @@ -209,7 +209,7 @@ impl VtableHelper { // SAFETY: The release call of a file owns the private data. let private =3D unsafe { (*file).private_data }; // SAFETY: The release call of a file owns the private data. - let ptr =3D unsafe { ::from_foreign(private)= }; + let ptr =3D unsafe { ::from_foreign(private.= cast()) }; =20 T::release(ptr); =20 @@ -227,7 +227,7 @@ impl VtableHelper { // SAFETY: The ioctl call of a file can access the private data. let private =3D unsafe { (*file).private_data }; // SAFETY: Ioctl calls can borrow the private data of the file. - let device =3D unsafe { ::borrow(private) }; + let device =3D unsafe { ::borrow(private.cas= t()) }; =20 match T::ioctl(device, cmd, arg as usize) { Ok(ret) =3D> ret as c_long, diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs index eb5cd8b360a3507a527978aaf96dbc3a80d4ae2c..8e29c332db86ae869d81f75de9c= 21fa73174de9a 100644 --- a/rust/kernel/sync/arc.rs +++ b/rust/kernel/sync/arc.rs @@ -130,9 +130,10 @@ pub struct Arc { _p: PhantomData>, } =20 +#[doc(hidden)] #[pin_data] #[repr(C)] -struct ArcInner { +pub struct ArcInner { refcount: Opaque, data: T, } @@ -330,18 +331,20 @@ pub fn into_unique_or_drop(self) -> Option>> { } } =20 -impl ForeignOwnable for Arc { +// SAFETY: The `into_foreign` function returns a pointer that is well-alig= ned. +unsafe impl ForeignOwnable for Arc { + type PointedTo =3D ArcInner; type Borrowed<'a> =3D ArcBorrow<'a, T>; type BorrowedMut<'a> =3D Self::Borrowed<'a>; =20 - fn into_foreign(self) -> *mut crate::ffi::c_void { - ManuallyDrop::new(self).ptr.as_ptr().cast() + fn into_foreign(self) -> *mut Self::PointedTo { + ManuallyDrop::new(self).ptr.as_ptr() } =20 - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self { + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { // SAFETY: The safety requirements of this function ensure that `p= tr` comes from a previous // call to `Self::into_foreign`. - let inner =3D unsafe { NonNull::new_unchecked(ptr.cast::>()) }; + let inner =3D unsafe { NonNull::new_unchecked(ptr) }; =20 // SAFETY: By the safety requirement of this function, we know tha= t `ptr` came from // a previous call to `Arc::into_foreign`, which guarantees that `= ptr` is valid and @@ -349,17 +352,17 @@ unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) = -> Self { unsafe { Self::from_inner(inner) } } =20 - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T>= { + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> { // SAFETY: The safety requirements of this function ensure that `p= tr` comes from a previous // call to `Self::into_foreign`. - let inner =3D unsafe { NonNull::new_unchecked(ptr.cast::>()) }; + let inner =3D unsafe { NonNull::new_unchecked(ptr) }; =20 // SAFETY: The safety requirements of `from_foreign` ensure that t= he object remains alive // for the lifetime of the returned value. unsafe { ArcBorrow::new(inner) } } =20 - unsafe fn borrow_mut<'a>(ptr: *mut core::ffi::c_void) -> ArcBorrow<'a,= T> { + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T= > { // SAFETY: The safety requirements for `borrow_mut` are a superset= of the safety // requirements for `borrow`. unsafe { Self::borrow(ptr) } diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs index 0dfaf45a755c7ce702027918e5fd3e97c407fda4..0cf93c293b884004a6ed64c2c09= 723efa7986270 100644 --- a/rust/kernel/types.rs +++ b/rust/kernel/types.rs @@ -18,7 +18,19 @@ /// /// This trait is meant to be used in cases when Rust objects are stored i= n C objects and /// eventually "freed" back to Rust. -pub trait ForeignOwnable: Sized { +/// +/// # Safety +/// +/// Implementers must ensure that [`into_foreign`] returns a pointer which= meets the alignment +/// requirements of [`PointedTo`]. +/// +/// [`into_foreign`]: Self::into_foreign +/// [`PointedTo`]: Self::PointedTo +pub unsafe trait ForeignOwnable: Sized { + /// Type used when the value is foreign-owned. In practical terms only= defines the alignment of + /// the pointer. + type PointedTo; + /// Type used to immutably borrow a value that is currently foreign-ow= ned. type Borrowed<'a>; =20 @@ -27,16 +39,18 @@ pub trait ForeignOwnable: Sized { =20 /// Converts a Rust-owned object to a foreign-owned one. /// - /// The foreign representation is a pointer to void. There are no guar= antees for this pointer. - /// For example, it might be invalid, dangling or pointing to uninitia= lized memory. Using it in - /// any way except for [`from_foreign`], [`try_from_foreign`], [`borro= w`], or [`borrow_mut`] can - /// result in undefined behavior. + /// # Guarantees + /// + /// The return value is guaranteed to be well-aligned, but there are n= o other guarantees for + /// this pointer. For example, it might be null, dangling, or point to= uninitialized memory. + /// Using it in any way except for [`ForeignOwnable::from_foreign`], [= `ForeignOwnable::borrow`], + /// [`ForeignOwnable::try_from_foreign`] can result in undefined behav= ior. /// /// [`from_foreign`]: Self::from_foreign /// [`try_from_foreign`]: Self::try_from_foreign /// [`borrow`]: Self::borrow /// [`borrow_mut`]: Self::borrow_mut - fn into_foreign(self) -> *mut crate::ffi::c_void; + fn into_foreign(self) -> *mut Self::PointedTo; =20 /// Converts a foreign-owned object back to a Rust-owned one. /// @@ -46,7 +60,7 @@ pub trait ForeignOwnable: Sized { /// must not be passed to `from_foreign` more than once. /// /// [`into_foreign`]: Self::into_foreign - unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self; + unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self; =20 /// Tries to convert a foreign-owned object back to a Rust-owned one. /// @@ -58,7 +72,7 @@ pub trait ForeignOwnable: Sized { /// `ptr` must either be null or satisfy the safety requirements for [= `from_foreign`]. /// /// [`from_foreign`]: Self::from_foreign - unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option { + unsafe fn try_from_foreign(ptr: *mut Self::PointedTo) -> Option { if ptr.is_null() { None } else { @@ -81,7 +95,7 @@ unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) = -> Option { /// /// [`into_foreign`]: Self::into_foreign /// [`from_foreign`]: Self::from_foreign - unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Self::Borrowed<'= a>; + unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> Self::Borrowed<'a>; =20 /// Borrows a foreign-owned object mutably. /// @@ -109,21 +123,23 @@ unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_vo= id) -> Option { /// [`from_foreign`]: Self::from_foreign /// [`borrow`]: Self::borrow /// [`Arc`]: crate::sync::Arc - unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Self::Borrow= edMut<'a>; + unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> Self::BorrowedM= ut<'a>; } =20 -impl ForeignOwnable for () { +// SAFETY: The `into_foreign` function returns a pointer that is dangling,= but well-aligned. +unsafe impl ForeignOwnable for () { + type PointedTo =3D (); type Borrowed<'a> =3D (); type BorrowedMut<'a> =3D (); =20 - fn into_foreign(self) -> *mut crate::ffi::c_void { + fn into_foreign(self) -> *mut Self::PointedTo { core::ptr::NonNull::dangling().as_ptr() } =20 - unsafe fn from_foreign(_: *mut crate::ffi::c_void) -> Self {} + unsafe fn from_foreign(_: *mut Self::PointedTo) -> Self {} =20 - unsafe fn borrow<'a>(_: *mut crate::ffi::c_void) -> Self::Borrowed<'a>= {} - unsafe fn borrow_mut<'a>(_: *mut crate::ffi::c_void) -> Self::Borrowed= Mut<'a> {} + unsafe fn borrow<'a>(_: *mut Self::PointedTo) -> Self::Borrowed<'a> {} + unsafe fn borrow_mut<'a>(_: *mut Self::PointedTo) -> Self::BorrowedMut= <'a> {} } =20 /// Runs a cleanup function/closure when dropped. --=20 2.47.0 From nobody Thu Dec 5 02:13:45 2024 Received: from mail-qk1-f180.google.com (mail-qk1-f180.google.com [209.85.222.180]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 79DB120A5F3; Tue, 3 Dec 2024 22:44:05 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.180 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733265847; cv=none; b=I8twvt+o2obedcxSjVojgX2ZP1OK+8zhuTbBkcHtctL/rBf6ixblj4cvyueuIYTYGJjmDz70gkAmbOvHEym97IKJ7qrQIziuw/maAtRJtGP5pm5FCeDudNVqjzTDi4yla/ebvy4VnUX42w/F0Jl9DSQcIj5n4JFTelP3ZsQja2w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1733265847; c=relaxed/simple; bh=vlpp608deHBVm9pmwKr4gaDRlJG/XpAmVzCmHV4zYDw=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=hE4VJg8poUhb9f0LKyiOtPJDKcjqz5oMW/lVO8AlSYAruofROOsaClcp/YgRf0wxeKws/ouzsk2BcuD4r+T0fS1rTZIXq18S7URil7XdN7pj1AgKn4kuTeIK6Wmoi5gCGumKm1IBRCpHhZ4p092T4ibnqhed/EjieuBso0PJ+ng= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=RYIA55IA; arc=none smtp.client-ip=209.85.222.180 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="RYIA55IA" Received: by mail-qk1-f180.google.com with SMTP id af79cd13be357-7b15d330ce1so445547185a.1; Tue, 03 Dec 2024 14:44:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1733265844; x=1733870644; darn=vger.kernel.org; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:from:to:cc:subject:date:message-id :reply-to; bh=vVht05H41kDrAIenOKnHFxrAEPbwV154pA5sGhrtS18=; b=RYIA55IAHMzGjDUGiayCkfHEbRwOFD83nH/VYY2mrP88P6K9xyJkEJe5oF9kZ7AVGe KkP+5mDSkfFQyobn/9LZ3yFUfNNjDKQ3/vDQiHVjGWJGuS29wCiXhB1Gee0u2VVW2j4l ylV+bCKXwxJBk1XPCqRwxGWw1eKCphvaVhl/4JtrZFpzt0V8qRgC/szwlDHbxB+4Kzz/ fOW/+kJ3M5DoRZKXBbbThaQnmVNNkAAinUj0uRUK5Wlgv8j0fqYcwDvW88cggPYa2HXv Vp/QCVgrRmQ0soC0X1bOQPoRzHZ4zaiZqw6xXbaxMwDKz+ORj1Hfc6i3AVKF53SXCZI4 kFXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1733265844; x=1733870644; h=cc:to:in-reply-to:references:message-id:content-transfer-encoding :mime-version:subject:date:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=vVht05H41kDrAIenOKnHFxrAEPbwV154pA5sGhrtS18=; b=k5ezLauxiSUyaHCGuRv/MEiePQCSgXzAhIvNef56jcb/1TuJkWD4IsR6KFENDDC42Z bYLGE3X7Yc/78XqLv2+rU6iIeUnCRk+NkWXpKoLlhDw2T0qnW7cT7f3R77MUDtd+WotY gw1/BeaOEjWmzCwByZ1TpX0sIuKKG1KrPaL3EfNnmvUnCYrCl9Zr7JAxHHd9hjGqhk/3 G4NrA1FlT09tAhMXG45lud5rA6vogrTw6P7Q01DuCb8Ueeb3MIssuwAPrtLuDqMNYbsC aulSMPywkTZgpMx4dWWv9p0t5rHU+JSbqA3qN5Mc3vkkY6uP/960OpvZTF4AxfkZ5eqv z5JQ== X-Forwarded-Encrypted: i=1; AJvYcCUjujENfmuVVBDas2Gym/x1hTsRhZsQyQfnl4FVQUKdbuaoj2vIc8XBkJkYawj1cwvZXzIQyvOpALZRJY1C8B4=@vger.kernel.org, AJvYcCWZ8cc/SaO5jzLFh84EnB5/IKZCerh7PiwUdKUAqgytDQXSdQZHWVl/vGVaZWLeoKghEtXtOmhXIPlLUbk=@vger.kernel.org X-Gm-Message-State: AOJu0YwY+Qxfqa81/wk93XWIkhDj8hh27XPu3SRIHh35K4/j3u25aaPN z5pFvY8gGOhzuodT2eL57zwQ1+7CVIRl1pzAD4+k7aIMLo2ml+DF X-Gm-Gg: ASbGncu/NxyImQ3q4XVoBRoBafndCS+gH4ABMlubQ1OR2U+iZi1VcD6tqLzV3ygRKTW 5ciDYTqjM1QyV/To4fwkRguHlZ/k8Ukr58dwmYHwkdT6E+f9G6xU66dzZ6ZYP61A0l7tqGayNDy MtiBFbAOnsaBgisNhfw7gXMd4cW4p2tWrKdtf3+ZxF4NYytSBjxWihgI1Rx+0MyM8St1GHn9XVf KFwG3ZewaaVSIw4nHqERpSL1atLbA5rD2Fmttknt+lF7CQddVw+5qcuK+beoPSDDt4wg34RHeis TS3pgf5qxzFQaUAERWaepwTsvpFJoewz5uG70YrC5NiM/AhGo5Rx9A== X-Google-Smtp-Source: AGHT+IGVJ20mtbdMqm9o6lxIZOGMPafHJOkq0ccuS/xP6KJQt9xHRToBPWUeASmT1K91dU467gPttg== X-Received: by 2002:a05:620a:17a1:b0:7b6:5dbe:1b52 with SMTP id af79cd13be357-7b6a5d77eb0mr575097685a.33.1733265844246; Tue, 03 Dec 2024 14:44:04 -0800 (PST) Received: from 1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa ([2620:10d:c091:600::1:352d]) by smtp.gmail.com with ESMTPSA id af79cd13be357-7b6849495d2sm555516185a.68.2024.12.03.14.44.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Dec 2024 14:44:03 -0800 (PST) From: Tamir Duberstein Date: Tue, 03 Dec 2024 17:43:51 -0500 Subject: [PATCH v11 2/2] rust: xarray: Add an abstraction for XArray Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Message-Id: <20241203-rust-xarray-bindings-v11-2-58a95d137ec2@gmail.com> References: <20241203-rust-xarray-bindings-v11-0-58a95d137ec2@gmail.com> In-Reply-To: <20241203-rust-xarray-bindings-v11-0-58a95d137ec2@gmail.com> To: Danilo Krummrich , Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , =?utf-8?q?Bj=C3=B6rn_Roy_Baron?= , Benno Lossin , Andreas Hindborg , Alice Ryhl , Trevor Gross Cc: =?utf-8?q?Ma=C3=ADra_Canal?= , Asahi Lina , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Tamir Duberstein X-Mailer: b4 0.15-dev `XArray` is an efficient sparse array of pointers. Add a Rust abstraction for this type. This implementation bounds the element type on `ForeignOwnable` and requires explicit locking for all operations. Future work may leverage RCU to enable lockless operation. Inspired-by: Ma=C3=ADra Canal Inspired-by: Asahi Lina Signed-off-by: Tamir Duberstein Reviewed-by: Andreas Hindborg --- rust/bindings/bindings_helper.h | 6 + rust/helpers/helpers.c | 1 + rust/helpers/xarray.c | 28 +++++ rust/kernel/alloc.rs | 5 + rust/kernel/lib.rs | 1 + rust/kernel/xarray.rs | 270 ++++++++++++++++++++++++++++++++++++= ++++ 6 files changed, 311 insertions(+) diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helpe= r.h index 5c4dfe22f41a5a106330e8c43ffbd342c69c4e0b..9f39d673b240281aed2759b5bd0= 76aa43fb54951 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -30,6 +30,7 @@ #include #include #include +#include #include =20 /* `bindgen` gets confused at certain things. */ @@ -43,3 +44,8 @@ const gfp_t RUST_CONST_HELPER___GFP_ZERO =3D __GFP_ZERO; const gfp_t RUST_CONST_HELPER___GFP_HIGHMEM =3D ___GFP_HIGHMEM; const gfp_t RUST_CONST_HELPER___GFP_NOWARN =3D ___GFP_NOWARN; const blk_features_t RUST_CONST_HELPER_BLK_FEAT_ROTATIONAL =3D BLK_FEAT_RO= TATIONAL; + +const xa_mark_t RUST_CONST_HELPER_XA_PRESENT =3D XA_PRESENT; + +const gfp_t RUST_CONST_HELPER_XA_FLAGS_ALLOC =3D XA_FLAGS_ALLOC; +const gfp_t RUST_CONST_HELPER_XA_FLAGS_ALLOC1 =3D XA_FLAGS_ALLOC1; diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c index dcf827a61b52e71e46fd5378878602eef5e538b6..ff28340e78c53c79baf18e2927c= c90350d8ab513 100644 --- a/rust/helpers/helpers.c +++ b/rust/helpers/helpers.c @@ -30,3 +30,4 @@ #include "vmalloc.c" #include "wait.c" #include "workqueue.c" +#include "xarray.c" diff --git a/rust/helpers/xarray.c b/rust/helpers/xarray.c new file mode 100644 index 0000000000000000000000000000000000000000..60b299f11451d2c4a75e50e25de= c4dac13f143f4 --- /dev/null +++ b/rust/helpers/xarray.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include + +int rust_helper_xa_err(void *entry) +{ + return xa_err(entry); +} + +void rust_helper_xa_init_flags(struct xarray *xa, gfp_t flags) +{ + return xa_init_flags(xa, flags); +} + +int rust_helper_xa_trylock(struct xarray *xa) +{ + return xa_trylock(xa); +} + +void rust_helper_xa_lock(struct xarray *xa) +{ + return xa_lock(xa); +} + +void rust_helper_xa_unlock(struct xarray *xa) +{ + return xa_unlock(xa); +} diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs index f2f7f3a53d298cf899e062346202ba3285ce3676..be9f164ece2e0fe71143e020124= 7d2b70c193c51 100644 --- a/rust/kernel/alloc.rs +++ b/rust/kernel/alloc.rs @@ -39,6 +39,11 @@ pub struct Flags(u32); =20 impl Flags { + /// Get a flags value with all bits unset. + pub fn empty() -> Self { + Self(0) + } + /// Get the raw representation of this flag. pub(crate) fn as_raw(self) -> u32 { self.0 diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index e1065a7551a39e68d6379031d80d4be336e652a3..9ca524b15920c525c7db419e60d= ec4c43522751d 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -68,6 +68,7 @@ pub mod types; pub mod uaccess; pub mod workqueue; +pub mod xarray; =20 #[doc(hidden)] pub use bindings; diff --git a/rust/kernel/xarray.rs b/rust/kernel/xarray.rs new file mode 100644 index 0000000000000000000000000000000000000000..39163ea037cb393a7c32a40b3e6= 539be33986b45 --- /dev/null +++ b/rust/kernel/xarray.rs @@ -0,0 +1,270 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! XArray abstraction. +//! +//! C header: [`include/linux/xarray.h`](srctree/include/linux/xarray.h) + +use crate::{ + alloc, bindings, build_assert, build_error, + error::{Error, Result}, + init::PinInit, + pin_init, + types::{ForeignOwnable, NotThreadSafe, Opaque}, +}; +use core::{iter, marker::PhantomData, mem, pin::Pin}; +use macros::{pin_data, pinned_drop}; + +/// An array which efficiently maps sparse integer indices to owned object= s. +/// +/// This is similar to a [`crate::alloc::kvec::Vec>`], but more = efficient when there are +/// holes in the index space, and can be efficiently grown. +/// +/// # Invariants +/// +/// `self.xa` is always an initialized and valid [`bindings::xarray`] whos= e entries are either +/// `XA_ZERO_ENTRY` or came from `T::into_foreign`. +/// +/// # Examples +/// +/// ```rust +/// use kernel::alloc::KBox; +/// use kernel::xarray::{AllocKind, XArray}; +/// +/// let xa =3D KBox::pin_init(XArray::new(AllocKind::Alloc1), GFP_KERNEL)?; +/// +/// let dead =3D KBox::new(0xdead, GFP_KERNEL)?; +/// let beef =3D KBox::new(0xbeef, GFP_KERNEL)?; +/// +/// let mut guard =3D xa.lock(); +/// +/// assert_eq!(guard.get(0), None); +/// +/// assert_eq!(guard.store(0, dead, GFP_KERNEL).unwrap().as_deref(), None); +/// assert_eq!(guard.get(0).copied(), Some(0xdead)); +/// +/// *guard.get_mut(0).unwrap() =3D 0xffff; +/// assert_eq!(guard.get(0).copied(), Some(0xffff)); +/// +/// assert_eq!(guard.store(0, beef, GFP_KERNEL).unwrap().as_deref().copied= (), Some(0xffff)); +/// assert_eq!(guard.get(0).copied(), Some(0xbeef)); +/// +/// guard.remove(0); +/// assert_eq!(guard.get(0), None); +/// +/// # Ok::<(), Error>(()) +/// ``` +#[pin_data(PinnedDrop)] +pub struct XArray { + #[pin] + xa: Opaque, + _p: PhantomData, +} + +#[pinned_drop] +impl PinnedDrop for XArray { + fn drop(self: Pin<&mut Self>) { + self.iter().for_each(|ptr| { + let ptr =3D ptr.as_ptr(); + // SAFETY: `ptr` came from `T::into_foreign`. + // + // INVARIANT: we own the only reference to the array which is = being dropped so the + // broken invariant is not observable on function exit. + drop(unsafe { T::from_foreign(ptr) }) + }); + + // SAFETY: `self.xa` is always valid by the type invariant. + unsafe { bindings::xa_destroy(self.xa.get()) }; + } +} + +/// Flags passed to [`XArray::new`] to configure the array's allocation tr= acking behavior. +pub enum AllocKind { + /// Consider the first element to be at index 0. + Alloc, + /// Consider the first element to be at index 1. + Alloc1, +} + +impl XArray { + /// Creates a new [`XArray`]. + pub fn new(kind: AllocKind) -> impl PinInit { + let flags =3D match kind { + AllocKind::Alloc =3D> bindings::XA_FLAGS_ALLOC, + AllocKind::Alloc1 =3D> bindings::XA_FLAGS_ALLOC1, + }; + pin_init!(Self { + // SAFETY: `xa` is valid while the closure is called. + xa <- Opaque::ffi_init(|xa| unsafe { + bindings::xa_init_flags(xa, flags) + }), + _p: PhantomData, + }) + } + + fn iter(&self) -> impl Iterator> + '_ { + // TODO: Remove when https://lore.kernel.org/all/20240913213041.39= 5655-5-gary@garyguo.net/ is applied. + const MAX: core::ffi::c_ulong =3D core::ffi::c_ulong::MAX; + + let mut index =3D 0; + + // SAFETY: `self.xa` is always valid by the type invariant. + iter::once(unsafe { + bindings::xa_find(self.xa.get(), &mut index, MAX, bindings::XA= _PRESENT) + }) + .chain(iter::from_fn(move || { + // SAFETY: `self.xa` is always valid by the type invariant. + Some(unsafe { + bindings::xa_find_after(self.xa.get(), &mut index, MAX, bi= ndings::XA_PRESENT) + }) + })) + .map_while(|ptr| core::ptr::NonNull::new(ptr.cast())) + } + + /// Attempts to lock the [`XArray`] for exclusive access. + pub fn try_lock(&self) -> Option> { + // SAFETY: `self.xa` is always valid by the type invariant. + (unsafe { bindings::xa_trylock(self.xa.get()) } !=3D 0).then(|| Gu= ard { + xa: self, + _not_send: NotThreadSafe, + }) + } + + /// Locks the [`XArray`] for exclusive access. + pub fn lock(&self) -> Guard<'_, T> { + // SAFETY: `self.xa` is always valid by the type invariant. + unsafe { bindings::xa_lock(self.xa.get()) }; + + Guard { + xa: self, + _not_send: NotThreadSafe, + } + } +} + +/// A lock guard. +/// +/// The lock is unlocked when the guard goes out of scope. +#[must_use =3D "the lock unlocks immediately when the guard is unused"] +pub struct Guard<'a, T: ForeignOwnable> { + xa: &'a XArray, + _not_send: NotThreadSafe, +} + +impl Drop for Guard<'_, T> { + fn drop(&mut self) { + // SAFETY: + // - `self.xa.xa` is always valid by the type invariant. + // - The caller holds the lock, so it is safe to unlock it. + unsafe { bindings::xa_unlock(self.xa.xa.get()) }; + } +} + +// TODO: Remove when https://lore.kernel.org/all/20240913213041.395655-5-g= ary@garyguo.net/ is applied. +fn to_index(i: usize) -> core::ffi::c_ulong { + i.try_into() + .unwrap_or_else(|_| build_error!("cannot convert usize to c_ulong"= )) +} + +impl<'a, T: ForeignOwnable> Guard<'a, T> { + fn load(&self, index: usize, f: F) -> Option + where + F: FnOnce(core::ptr::NonNull) -> U, + { + // SAFETY: `self.xa.xa` is always valid by the type invariant. + let ptr =3D unsafe { bindings::xa_load(self.xa.xa.get(), to_index(= index)) }; + let ptr =3D core::ptr::NonNull::new(ptr.cast())?; + Some(f(ptr)) + } + + /// Loads an entry from the array. + /// + /// Returns the entry at the given index. + pub fn get(&self, index: usize) -> Option> { + self.load(index, |ptr| { + // SAFETY: `ptr` came from `T::into_foreign`. + unsafe { T::borrow(ptr.as_ptr()) } + }) + } + + /// Loads an entry from the array. + /// + /// Returns the entry at the given index. + pub fn get_mut(&mut self, index: usize) -> Option> { + self.load(index, |ptr| { + // SAFETY: `ptr` came from `T::into_foreign`. + unsafe { T::borrow_mut(ptr.as_ptr()) } + }) + } + + /// Erases an entry from the array. + /// + /// Returns the entry which was previously at the given index. + pub fn remove(&mut self, index: usize) -> Option { + // SAFETY: `self.xa.xa` is always valid by the type invariant. + // + // SAFETY: The caller holds the lock. + let ptr =3D unsafe { bindings::__xa_erase(self.xa.xa.get(), to_ind= ex(index)) }.cast(); + // SAFETY: `ptr` is either NULL or came from `T::into_foreign`. + unsafe { T::try_from_foreign(ptr) } + } + + /// Stores an entry in the array. + /// + /// May drop the lock if needed to allocate memory, and then reacquire= it afterwards. + /// + /// On success, returns the entry which was previously at the given in= dex. + /// + /// On failure, returns the entry which was attempted to be stored. + pub fn store( + &mut self, + index: usize, + value: T, + gfp: alloc::Flags, + ) -> Result, (T, Error)> { + build_assert!( + mem::align_of::() >=3D 4, + "pointers stored in XArray must be 4-byte aligned" + ); + let new =3D value.into_foreign(); + + let old =3D { + let new =3D new.cast(); + // SAFETY: `self.xa.xa` is always valid by the type invariant. + // + // SAFETY: The caller holds the lock. + // + // INVARIANT: `new` came from `T::into_foreign`. + unsafe { bindings::__xa_store(self.xa.xa.get(), to_index(index= ), new, gfp.as_raw()) } + }; + + // SAFETY: `__xa_store` returns the old entry at this index on suc= cess or `xa_err` if an + // error happened. + match unsafe { bindings::xa_err(old) } { + 0 =3D> { + let old =3D old.cast(); + // SAFETY: `ptr` is either NULL or came from `T::into_fore= ign`. + // + // NB: `XA_ZERO_ENTRY` is never returned by functions belo= nging to the Normal XArray + // API; such entries present as `NULL`. + Ok(unsafe { T::try_from_foreign(old) }) + } + errno =3D> { + // SAFETY: `new` came from `T::into_foreign` and `__xa_sto= re` does not take + // ownership of the value on error. + let value =3D unsafe { T::from_foreign(new) }; + Err((value, Error::from_errno(errno))) + } + } + } +} + +// SAFETY: It is safe to send `XArray` to another thread when the under= lying `T` is `Send` +// because XArray is thread-safe and all mutation operations are synchroni= zed. +unsafe impl Send for XArray {} + +// SAFETY: It is safe to send `&XArray` to another thread when the unde= rlying `T` is `Sync` +// because it effectively means sharing `&T` (which is safe because `T` is= `Sync`); additionally, it +// needs `T` to be `Send` because any thread that has a `&XArray` may l= ock it and get a +// `Guard` on that thread, so the thread may ultimately access `T` usin= g a mutable reference, for +// example, using `get_mut` or `remove`. +unsafe impl Sync for XArray {} --=20 2.47.0