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 <[email protected]>
---
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.
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
On 1/29/24 11:58, 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 <[email protected]>
> ---
> [...]
Reviewed-by: Martin Rodriguez Reboredo <[email protected]>
On Mon, Jan 29, 2024 at 9:58 AM Alice Ryhl <[email protected]> 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 <[email protected]>
> ---
Reviewed-by: Trevor Gross <[email protected]>
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 <[email protected]>
> ---
> 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 <[email protected]>
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
>
>
On Mon, Jan 29, 2024 at 3:58 PM Alice Ryhl <[email protected]> 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 <[email protected]>
Applied to `rust-next` with the new `prelude`s removed and a slightly
reworded title -- thanks everyone!
(I also created a "good first issue" about other `prelude`s that can
be removed: https://github.com/Rust-for-Linux/linux/issues/1064)
Cheers,
Miguel