On 11/29/23 14:12, Alice Ryhl wrote: > diff --git a/rust/kernel/file.rs b/rust/kernel/file.rs > index 578ee307093f..35576678c993 100644 > --- a/rust/kernel/file.rs > +++ b/rust/kernel/file.rs > @@ -14,6 +14,9 @@ > use alloc::boxed::Box; > use core::{alloc::AllocError, marker::PhantomData, mem, ptr}; > > +mod poll_table; > +pub use self::poll_table::{PollCondVar, PollTable}; I think it makes more sense to put it under `rust/kernel/sync/`. > + fn get_qproc(&self) -> bindings::poll_queue_proc { > + let ptr = self.0.get(); > + // SAFETY: The `ptr` is valid because it originates from a reference, and the `_qproc` > + // field is not modified concurrently with this call. What ensures this? Maybe use a type invariant? > + unsafe { (*ptr)._qproc } > + } [...] > +impl PollCondVar { > + /// Constructs a new condvar initialiser. > + #[allow(clippy::new_ret_no_self)] This is no longer needed, as Gary fixed this, see [1]. [1]: https://github.com/rust-lang/rust-clippy/issues/7344 > + pub fn new(name: &'static CStr, key: &'static LockClassKey) -> impl PinInit<Self> { > + pin_init!(Self { > + inner <- CondVar::new(name, key), > + }) > + } > +} > + > +// Make the `CondVar` methods callable on `PollCondVar`. > +impl Deref for PollCondVar { > + type Target = CondVar; > + > + fn deref(&self) -> &CondVar { > + &self.inner > + } > +} > + > +#[pinned_drop] > +impl PinnedDrop for PollCondVar { > + fn drop(self: Pin<&mut Self>) { > + // Clear anything registered using `register_wait`. > + self.inner.notify(1, bindings::POLLHUP | bindings::POLLFREE); Isn't notifying only a single thread problematic, since a user could misuse the `PollCondVar` (since all functions of `CondVar` are also accessible) and also `.wait()` on the condvar? When dropping a `PollCondVar` it might notify only the user `.wait()`, but not the `PollTable`. Or am I missing something? -- Cheers, Benno > + // Wait for epoll items to be properly removed. > + // > + // SAFETY: Just an FFI call. > + unsafe { bindings::synchronize_rcu() }; > + } > +}