Received-SPF: pass (google.com: domain of linux-kernel+bounces-53132-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Feedback-ID: iad51458e:Fastmail
Date: Mon, 5 Feb 2024 09:39:15 -0800
From: Boqun Feng <boqun.feng@gmail.com>
To: Alice Ryhl <aliceryhl@google.com>
Cc: Miguel Ojeda <ojeda@kernel.org>,
	Wedson Almeida Filho <wedsonaf@gmail.com>,
	Alex Gaynor <alex.gaynor@gmail.com>, Gary Guo <gary@garyguo.net>,
	=?iso-8859-1?Q?Bj=F6rn?= Roy Baron <bjorn3_gh@protonmail.com>,
	Benno Lossin <benno.lossin@proton.me>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] rust: add reexports for macros
Message-ID: <ZcEdQyaz0uLDOhrk@boqun-archlinux>
References: <20240129145837.1419880-1-aliceryhl@google.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240129145837.1419880-1-aliceryhl@google.com>

On Mon, Jan 29, 2024 at 02:58:37PM +0000, Alice Ryhl wrote:
> Currently, all macros are reexported with #[macro_export] only, which
> means that to access `new_work!` from the workqueue, you need to import
> it from the path `kernel::new_work` instead of importing it from the
> workqueue module like all other items in the workqueue. By adding
> reexports of the macros, it becomes possible to import the macros from
> the correct modules.
> 
> It's still possible to import the macros from the root, but I don't
> think we can do anything about that.
> 
> There is no functional change. This is merely a code cleanliness
> improvement.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> I hope that the KUnit examples still compile. I tried checking myself
> and it seemed to work, but I'm not completely sure whether I used the
> tool correctly. I would appreciate if someone is able to double-check
> and give a Tested-by for that.

Tested-by: Boqun Feng <boqun.feng@gmail.com>

Regards,
Boqun

> 
>  rust/kernel/init.rs               |  8 +++++---
>  rust/kernel/sync.rs               |  5 +++--
>  rust/kernel/sync/condvar.rs       |  4 ++--
>  rust/kernel/sync/lock/mutex.rs    |  4 +++-
>  rust/kernel/sync/lock/spinlock.rs |  4 +++-
>  rust/kernel/workqueue.rs          | 14 ++++++--------
>  6 files changed, 22 insertions(+), 17 deletions(-)
> 
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 65be9ae57b80..d7107db1d587 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -36,7 +36,8 @@
>  //!
>  //! ```rust
>  //! # #![allow(clippy::disallowed_names)]
> -//! use kernel::{prelude::*, sync::Mutex, new_mutex};
> +//! use kernel::prelude::*;
> +//! use kernel::sync::{new_mutex, Mutex};
>  //! # use core::pin::Pin;
>  //! #[pin_data]
>  //! struct Foo {
> @@ -56,7 +57,8 @@
>  //!
>  //! ```rust
>  //! # #![allow(clippy::disallowed_names)]
> -//! # use kernel::{prelude::*, sync::Mutex, new_mutex};
> +//! # use kernel::prelude::*;
> +//! # use kernel::sync::{new_mutex, Mutex};
>  //! # use core::pin::Pin;
>  //! # #[pin_data]
>  //! # struct Foo {
> @@ -79,7 +81,7 @@
>  //! above method only works for types where you can access the fields.
>  //!
>  //! ```rust
> -//! # use kernel::{new_mutex, sync::{Arc, Mutex}};
> +//! # use kernel::sync::{new_mutex, Arc, Mutex};
>  //! let mtx: Result<Arc<Mutex<usize>>> = Arc::pin_init(new_mutex!(42, "example::mtx"));
>  //! ```
>  //!
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index c1fb10fc64f4..c983f63fd56e 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -13,8 +13,9 @@
>  mod locked_by;
>  
>  pub use arc::{Arc, ArcBorrow, UniqueArc};
> -pub use condvar::{CondVar, CondVarTimeoutResult};
> -pub use lock::{mutex::Mutex, spinlock::SpinLock};
> +pub use condvar::{new_condvar, CondVar, CondVarTimeoutResult};
> +pub use lock::mutex::{new_mutex, Mutex};
> +pub use lock::spinlock::{new_spinlock, SpinLock};
>  pub use locked_by::LockedBy;
>  
>  /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
> diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
> index 9f1d83589beb..72680ff57e67 100644
> --- a/rust/kernel/sync/condvar.rs
> +++ b/rust/kernel/sync/condvar.rs
> @@ -27,6 +27,7 @@ macro_rules! new_condvar {
>          $crate::sync::CondVar::new($crate::optional_name!($($name)?), $crate::static_lock_class!())
>      };
>  }
> +pub use new_condvar;
>  
>  /// A conditional variable.
>  ///
> @@ -44,8 +45,7 @@ macro_rules! new_condvar {
>  /// The following is an example of using a condvar with a mutex:
>  ///
>  /// ```
> -/// use kernel::sync::{CondVar, Mutex};
> -/// use kernel::{new_condvar, new_mutex};
> +/// use kernel::sync::{new_condvar, new_mutex, CondVar, Mutex};
>  ///
>  /// #[pin_data]
>  /// pub struct Example {
> diff --git a/rust/kernel/sync/lock/mutex.rs b/rust/kernel/sync/lock/mutex.rs
> index 8c524a3ec45a..38207f9ce78a 100644
> --- a/rust/kernel/sync/lock/mutex.rs
> +++ b/rust/kernel/sync/lock/mutex.rs
> @@ -17,6 +17,7 @@ macro_rules! new_mutex {
>              $inner, $crate::optional_name!($($name)?), $crate::static_lock_class!())
>      };
>  }
> +pub use new_mutex;
>  
>  /// A mutual exclusion primitive.
>  ///
> @@ -35,7 +36,8 @@ macro_rules! new_mutex {
>  /// contains an inner struct (`Inner`) that is protected by a mutex.
>  ///
>  /// ```
> -/// use kernel::{init::InPlaceInit, init::PinInit, new_mutex, pin_init, sync::Mutex};
> +/// use kernel::prelude::*;
> +/// use kernel::sync::{new_mutex, Mutex};
>  ///
>  /// struct Inner {
>  ///     a: u32,
> diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs
> index 068535ce1b29..cb2c6f71e80e 100644
> --- a/rust/kernel/sync/lock/spinlock.rs
> +++ b/rust/kernel/sync/lock/spinlock.rs
> @@ -17,6 +17,7 @@ macro_rules! new_spinlock {
>              $inner, $crate::optional_name!($($name)?), $crate::static_lock_class!())
>      };
>  }
> +pub use new_spinlock;
>  
>  /// A spinlock.
>  ///
> @@ -33,7 +34,8 @@ macro_rules! new_spinlock {
>  /// contains an inner struct (`Inner`) that is protected by a spinlock.
>  ///
>  /// ```
> -/// use kernel::{init::InPlaceInit, init::PinInit, new_spinlock, pin_init, sync::SpinLock};
> +/// use kernel::prelude::*;
> +/// use kernel::sync::{new_spinlock, SpinLock};
>  ///
>  /// struct Inner {
>  ///     a: u32,
> diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
> index 498397877376..0ed17451b06e 100644
> --- a/rust/kernel/workqueue.rs
> +++ b/rust/kernel/workqueue.rs
> @@ -35,8 +35,7 @@
>  //! ```
>  //! use kernel::prelude::*;
>  //! use kernel::sync::Arc;
> -//! use kernel::workqueue::{self, Work, WorkItem};
> -//! use kernel::{impl_has_work, new_work};
> +//! use kernel::workqueue::{self, impl_has_work, new_work, Work, WorkItem};
>  //!
>  //! #[pin_data]
>  //! struct MyStruct {
> @@ -78,8 +77,7 @@
>  //! ```
>  //! use kernel::prelude::*;
>  //! use kernel::sync::Arc;
> -//! use kernel::workqueue::{self, Work, WorkItem};
> -//! use kernel::{impl_has_work, new_work};
> +//! use kernel::workqueue::{self, impl_has_work, new_work, Work, WorkItem};
>  //!
>  //! #[pin_data]
>  //! struct MyStruct {
> @@ -147,6 +145,7 @@ macro_rules! new_work {
>          $crate::workqueue::Work::new($crate::optional_name!($($name)?), $crate::static_lock_class!())
>      };
>  }
> +pub use new_work;
>  
>  /// A kernel work queue.
>  ///
> @@ -396,9 +395,8 @@ pub unsafe fn raw_get(ptr: *const Self) -> *mut bindings::work_struct {
>  /// like this:
>  ///
>  /// ```no_run
> -/// use kernel::impl_has_work;
>  /// use kernel::prelude::*;
> -/// use kernel::workqueue::Work;
> +/// use kernel::workqueue::{impl_has_work, Work};
>  ///
>  /// struct MyWorkItem {
>  ///     work_field: Work<MyWorkItem, 1>,
> @@ -473,9 +471,8 @@ unsafe fn work_container_of(ptr: *mut Work<T, ID>) -> *mut Self
>  /// # Examples
>  ///
>  /// ```
> -/// use kernel::impl_has_work;
>  /// use kernel::sync::Arc;
> -/// use kernel::workqueue::{self, Work};
> +/// use kernel::workqueue::{self, impl_has_work, Work};
>  ///
>  /// struct MyStruct {
>  ///     work_field: Work<MyStruct, 17>,
> @@ -509,6 +506,7 @@ unsafe fn raw_get_work(ptr: *mut Self) -> *mut $crate::workqueue::Work<$work_typ
>          }
>      )*};
>  }
> +pub use impl_has_work;
>  
>  impl_has_work! {
>      impl<T> HasWork<Self> for ClosureWork<T> { self.work }
> 
> base-commit: f090f0d0eea9666a96702b29bc9a64cbabee85c5
> -- 
> 2.43.0.429.g432eaa2c6b-goog
> 
>