2024-01-05 01:38:52

by Charalampos Mitrodimas

[permalink] [raw]
Subject: [PATCH] rust: sync: `CondVar` rename "wait_list" to "wait_queue_head"

Fields named "wait_list" usually are of type "struct list_head". To
avoid confusion and because it is of type
"Opaque<bindings::wait_queue_head>" we are renaming "wait_list" to
"wait_queue_head".

Signed-off-by: Charalampos Mitrodimas <[email protected]>
---
rust/kernel/sync/condvar.rs | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index b679b6f6dbeb..ea0d559fbde5 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -73,7 +73,7 @@ macro_rules! new_condvar {
#[pin_data]
pub struct CondVar {
#[pin]
- pub(crate) wait_list: Opaque<bindings::wait_queue_head>,
+ pub(crate) wait_queue_head: Opaque<bindings::wait_queue_head>,

/// A condvar needs to be pinned because it contains a [`struct list_head`] that is
/// self-referential, so it cannot be safely moved once it is initialised.
@@ -96,7 +96,7 @@ pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self
_pin: PhantomPinned,
// SAFETY: `slot` is valid while the closure is called and both `name` and `key` have
// static lifetimes so they live indefinitely.
- wait_list <- Opaque::ffi_init(|slot| unsafe {
+ wait_queue_head <- Opaque::ffi_init(|slot| unsafe {
bindings::__init_waitqueue_head(slot, name.as_char_ptr(), key.as_ptr())
}),
})
@@ -108,16 +108,20 @@ fn wait_internal<T: ?Sized, B: Backend>(&self, wait_state: u32, guard: &mut Guar
// SAFETY: `wait` points to valid memory.
unsafe { bindings::init_wait(wait.get()) };

- // SAFETY: Both `wait` and `wait_list` point to valid memory.
+ // SAFETY: Both `wait` and `wait_queue_head` point to valid memory.
unsafe {
- bindings::prepare_to_wait_exclusive(self.wait_list.get(), wait.get(), wait_state as _)
+ bindings::prepare_to_wait_exclusive(
+ self.wait_queue_head.get(),
+ wait.get(),
+ wait_state as _,
+ )
};

// SAFETY: No arguments, switches to another thread.
guard.do_unlocked(|| unsafe { bindings::schedule() });

- // SAFETY: Both `wait` and `wait_list` point to valid memory.
- unsafe { bindings::finish_wait(self.wait_list.get(), wait.get()) };
+ // SAFETY: Both `wait` and `wait_queue_head` point to valid memory.
+ unsafe { bindings::finish_wait(self.wait_queue_head.get(), wait.get()) };
}

/// Releases the lock and waits for a notification in interruptible mode.
@@ -144,10 +148,10 @@ pub fn wait_uninterruptible<T: ?Sized, B: Backend>(&self, guard: &mut Guard<'_,

/// Calls the kernel function to notify the appropriate number of threads with the given flags.
fn notify(&self, count: i32, flags: u32) {
- // SAFETY: `wait_list` points to valid memory.
+ // SAFETY: `wait_queue_head` points to valid memory.
unsafe {
bindings::__wake_up(
- self.wait_list.get(),
+ self.wait_queue_head.get(),
bindings::TASK_NORMAL,
count,
flags as _,
--
2.39.2



Subject: Re: [PATCH] rust: sync: `CondVar` rename "wait_list" to "wait_queue_head"

On 1/4/24 22:29, Charalampos Mitrodimas wrote:
> Fields named "wait_list" usually are of type "struct list_head". To
> avoid confusion and because it is of type
> "Opaque<bindings::wait_queue_head>" we are renaming "wait_list" to
> "wait_queue_head".
>
> Signed-off-by: Charalampos Mitrodimas <[email protected]>
> ---
> [...]

Reviewed-by: Martin Rodriguez Reboredo <[email protected]>

2024-01-05 09:43:06

by Alice Ryhl

[permalink] [raw]
Subject: Re: [PATCH] rust: sync: `CondVar` rename "wait_list" to "wait_queue_head"

On Fri, Jan 5, 2024 at 2:29 AM Charalampos Mitrodimas
<[email protected]> wrote:
>
> Fields named "wait_list" usually are of type "struct list_head". To
> avoid confusion and because it is of type
> "Opaque<bindings::wait_queue_head>" we are renaming "wait_list" to
> "wait_queue_head".
>
> Signed-off-by: Charalampos Mitrodimas <[email protected]>

This has merge conflicts with a bunch of other patchsets, but the
change itself looks good.

Reviewed-by: Alice Ryhl <[email protected]>

2024-01-10 09:56:50

by Benno Lossin

[permalink] [raw]
Subject: Re: [PATCH] rust: sync: `CondVar` rename "wait_list" to "wait_queue_head"

On 05.01.24 02:29, Charalampos Mitrodimas wrote:
> Fields named "wait_list" usually are of type "struct list_head". To
> avoid confusion and because it is of type
> "Opaque<bindings::wait_queue_head>" we are renaming "wait_list" to
> "wait_queue_head".
>
> Signed-off-by: Charalampos Mitrodimas <[email protected]>
> ---
> rust/kernel/sync/condvar.rs | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)

Reviewed-by: Benno Lossin <[email protected]>

Thanks for the patch!

--
Cheers,
Benno



2024-01-22 15:22:30

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] rust: sync: `CondVar` rename "wait_list" to "wait_queue_head"

On Fri, Jan 5, 2024 at 2:29 AM Charalampos Mitrodimas
<[email protected]> wrote:
>
> Fields named "wait_list" usually are of type "struct list_head". To
> avoid confusion and because it is of type
> "Opaque<bindings::wait_queue_head>" we are renaming "wait_list" to
> "wait_queue_head".

Applied to `rust-next` -- thanks everyone!

Cheers,
Miguel