mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Gary Guo" <gary@garyguo.net>
To: "Philipp Stanner" <phasta@kernel.org>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@redhat.com>, "Will Deacon" <will@kernel.org>,
	"Boqun Feng" <boqun@kernel.org>,
	"Waiman Long" <longman@redhat.com>, "Gary Guo" <gary@garyguo.net>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Lyude Paul" <lyude@redhat.com>,
	"Daniel Almeida" <daniel.almeida@collabora.com>,
	"Onur Özkan" <work@onurozkan.dev>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Tamir Duberstein" <tamird@kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>
Cc: <linux-kernel@vger.kernel.org>, <rust-for-linux@vger.kernel.org>
Subject: Re: [PATCH] rust: sync: Accept errors for Lock payload data
Date: Thu, 24 Sep 2026 11:41:51 +0100	[thread overview]
Message-ID: <DLNHF9TPLXK2.1VQ2LJ0U6Y07K@garyguo.net> (raw)
In-Reply-To: <20260924085523.2620704-2-phasta@kernel.org>

On Thu Sep 24, 2026 at 9:55 AM BST, Philipp Stanner wrote:
> The `Lock` baseclass currently does not allow for fallible user-data.
> This can cause conflicts when used with other primitives, for example
> when a lock shall be pin-initialized with payload data that is fallible.
>
> Add support for the lock base class so that it works with
> try_pin_init!(). Adjust spinlock and mutex accordingly.
>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
> Regarding removal of the CStrExt, I got "unused import" errors because
> of it. Don't fully understand why?

Because it's part of the prelude.

>
> P.
> ---
>  rust/kernel/sync/lock.rs          | 21 ++++++++++++++-------
>  rust/kernel/sync/lock/mutex.rs    |  7 ++++---
>  rust/kernel/sync/lock/spinlock.rs | 20 ++++++++++----------
>  3 files changed, 28 insertions(+), 20 deletions(-)
>
> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> index 10b6b5e9b024..1b2df5d0dcf9 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -7,11 +7,13 @@
>  
>  use super::LockClassKey;
>  use crate::{
> -    str::{CStr, CStrExt as _},
> +    str::CStr,
>      types::{NotThreadSafe, Opaque, ScopeGuard},
>  };
>  use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin};
> -use pin_init::{pin_data, pin_init, PinInit, Wrapper};
> +use pin_init::{pin_data, PinInit, Wrapper};
> +
> +use kernel::prelude::*;
>  
>  pub mod mutex;
>  pub mod spinlock;
> @@ -126,14 +128,19 @@ unsafe impl<T: ?Sized + Send, B: Backend> Send for Lock<T, B> {}
>  // data it protects is `Send`.
>  unsafe impl<T: ?Sized + Send, B: Backend> Sync for Lock<T, B> {}
>  
> +use core::convert::Infallible;
> +
>  impl<T, B: Backend> Lock<T, B> {
>      /// Constructs a new lock initialiser.
> -    pub fn new(
> -        t: impl PinInit<T>,
> +    pub fn new<E>(
> +        t: impl PinInit<T, E>,
>          name: &'static CStr,
>          key: Pin<&'static LockClassKey>,
> -    ) -> impl PinInit<Self> {
> -        pin_init!(Self {
> +    ) -> impl PinInit<Self, E>
> +    where
> +        E: From<Infallible>,
> +    {
> +        try_pin_init!(Self {

FWIW if `? E` is specified, both `pin_init!` and `try_pin_init!` behave the
same, so there no need to change it here.

The difference between these two macros are the default error type; the former is
`Infallible` and the latter is `kernel::error::Error`.

>              data <- UnsafeCell::pin_init(t),
>              _pin: PhantomPinned,
>              // SAFETY: `slot` is valid while the closure is called and both `name` and `key` have
> @@ -141,7 +148,7 @@ pub fn new(
>              state <- Opaque::ffi_init(|slot| unsafe {
>                  B::init(slot, name.as_char_ptr(), key.as_ptr())
>              }),
> -        })
> +        }? E)
>      }
>  }
>  
> diff --git a/rust/kernel/sync/lock/mutex.rs b/rust/kernel/sync/lock/mutex.rs
> index cda0203efefb..35e6b02ff426 100644
> --- a/rust/kernel/sync/lock/mutex.rs
> +++ b/rust/kernel/sync/lock/mutex.rs
> @@ -35,6 +35,7 @@ macro_rules! new_mutex {
>  ///
>  /// ```
>  /// use kernel::sync::{new_mutex, Mutex};
> +/// use kernel::prelude::*;
>  ///
>  /// struct Inner {
>  ///     a: u32,
> @@ -49,8 +50,8 @@ macro_rules! new_mutex {
>  /// }
>  ///
>  /// impl Example {
> -///     fn new() -> impl PinInit<Self> {
> -///         pin_init!(Self {
> +///     fn new() -> impl PinInit<Self, Error> {
> +///         try_pin_init!(Self {
>  ///             c: 10,
>  ///             d <- new_mutex!(Inner { a: 20, b: 30 }),
>  ///         })
> @@ -58,7 +59,7 @@ macro_rules! new_mutex {
>  /// }
>  ///
>  /// // Allocate a boxed `Example`.
> -/// let e = KBox::pin_init(Example::new(), GFP_KERNEL)?;
> +/// let e = KBox::try_pin_init(Example::new(), GFP_KERNEL)?;
>  /// assert_eq!(e.c, 10);
>  /// assert_eq!(e.d.lock().a, 20);
>  /// assert_eq!(e.d.lock().b, 30);
> diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs
> index aafc80125f59..8ea2c5b202e6 100644
> --- a/rust/kernel/sync/lock/spinlock.rs
> +++ b/rust/kernel/sync/lock/spinlock.rs
> @@ -4,10 +4,7 @@
>  //!
>  //! This module allows Rust code to use the kernel's `spinlock_t`.
>  use super::*;
> -use crate::{
> -    interrupt::LocalInterruptDisabled,
> -    prelude::*, //
> -};
> +use crate::interrupt::LocalInterruptDisabled;
>  
>  /// Creates a [`SpinLock`] initialiser with the given name and a newly-created lock class.
>  ///
> @@ -38,6 +35,7 @@ macro_rules! new_spinlock {
>  ///
>  /// ```
>  /// use kernel::sync::{new_spinlock, SpinLock};
> +/// use kernel::prelude::*;

Doctests have prelude imported by default.

Best,
Gary


>  ///
>  /// struct Inner {
>  ///     a: u32,
> @@ -52,8 +50,8 @@ macro_rules! new_spinlock {
>  /// }
>  ///
>  /// impl Example {
> -///     fn new() -> impl PinInit<Self> {
> -///         pin_init!(Self {
> +///     fn new() -> impl PinInit<Self, Error> {
> +///         try_pin_init!(Self {
>  ///             c: 10,
>  ///             d <- new_spinlock!(Inner { a: 20, b: 30 }),
>  ///         })
> @@ -184,6 +182,7 @@ macro_rules! new_spinlock_irq {
>  ///
>  /// ```
>  /// use kernel::sync::{new_spinlock_irq, SpinLockIrq};
> +/// use kernel::prelude::*;
>  ///
>  /// struct Inner {
>  ///     a: u32,
> @@ -199,8 +198,8 @@ macro_rules! new_spinlock_irq {
>  /// }
>  ///
>  /// impl Example {
> -///     fn new() -> impl PinInit<Self> {
> -///         pin_init!(Self {
> +///     fn new() -> impl PinInit<Self, Error> {
> +///         try_pin_init!(Self {
>  ///             c <- new_spinlock_irq!(Inner { a: 0, b: 10 }),
>  ///             d <- new_spinlock_irq!(Inner { a: 20, b: 30 }),
>  ///         })
> @@ -232,6 +231,7 @@ macro_rules! new_spinlock_irq {
>  /// ```
>  /// use kernel::sync::{new_spinlock_irq, SpinLockIrq};
>  /// use kernel::interrupt::*;
> +/// use kernel::prelude::*;
>  ///
>  /// struct Inner {
>  ///     a: u32,
> @@ -244,8 +244,8 @@ macro_rules! new_spinlock_irq {
>  /// }
>  ///
>  /// impl Example {
> -///     fn new() -> impl PinInit<Self> {
> -///         pin_init!(Self {
> +///     fn new() -> impl PinInit<Self, Error> {
> +///         try_pin_init!(Self {
>  ///             inner <- new_spinlock_irq!(Inner { a: 20 }),
>  ///         })
>  ///     }



      reply	other threads:[~2026-09-24 10:41 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-24  8:55 Philipp Stanner
2026-09-24 10:41 ` Gary Guo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DLNHF9TPLXK2.1VQ2LJ0U6Y07K@garyguo.net \
    --to=gary@garyguo.net \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=longman@redhat.com \
    --cc=lossin@kernel.org \
    --cc=lyude@redhat.com \
    --cc=mingo@redhat.com \
    --cc=ojeda@kernel.org \
    --cc=peterz@infradead.org \
    --cc=phasta@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=will@kernel.org \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®