[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <DCBBFAC5-A4B4-41BA-8732-32FA96EDE28E@collabora.com>
Date: Mon, 21 Jul 2025 16:13:30 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Miguel Ojeda <ojeda@...nel.org>,
Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Andreas Hindborg <a.hindborg@...nel.org>,
Trevor Gross <tmgross@...ch.edu>,
Danilo Krummrich <dakr@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Krzysztof Wilczyński <kwilczynski@...nel.org>,
Benno Lossin <lossin@...nel.org>,
Dirk Behme <dirk.behme@...il.com>,
linux-kernel@...r.kernel.org,
rust-for-linux@...r.kernel.org,
linux-pci@...r.kernel.org
Subject: Re: [PATCH] rust: irq: add &Device<Bound> argument to irq callbacks
Alice,
> On 21 Jul 2025, at 11:38, Alice Ryhl <aliceryhl@...gle.com> wrote:
>
> When working with a bus device, many operations are only possible while
> the device is still bound. The &Device<Bound> type represents a proof in
> the type system that you are in a scope where the device is guaranteed
> to still be bound. Since we deregister irq callbacks when unbinding a
> device, if an irq callback is running, that implies that the device has
> not yet been unbound.
>
> To allow drivers to take advantage of that, add an additional argument
> to irq callbacks.
>
> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
> ---
> This patch is a follow-up to Daniel's irq series [1] that adds a
> &Device<Bound> argument to all irq callbacks. This allows you to use
> operations that are only safe on a bound device inside an irq callback.
>
> The patch is otherwise based on top of driver-core-next.
>
> [1]: https://lore.kernel.org/r/20250715-topics-tyr-request_irq2-v7-0-d469c0f37c07@collabora.com
I am having a hard time applying this locally.
> ///
> /// This function should be only used as the callback in `request_irq`.
> unsafe extern "C" fn handle_irq_callback<T: Handler>(_irq: i32, ptr: *mut c_void) -> c_uint {
> - // SAFETY: `ptr` is a pointer to T set in `Registration::new`
> - let handler = unsafe { &*(ptr as *const T) };
> - T::handle(handler) as c_uint
> + // SAFETY: `ptr` is a pointer to `Registration<T>` set in `Registration::new`
> + let registration = unsafe { &*(ptr as *const Registration<T>) };
> + // SAFETY: The irq callback is removed before the device is unbound, so the fact that the irq
> + // callback is running implies that the device has not yet been unbound.
> + let device = unsafe { registration.inner.device().as_bound() };
Where was this function introduced? i.e. I am missing the change that brought
in RegistrationInner::device(), or maybe some Deref impl that would make this
possible?
Also, I wonder if we can't make the scope of this unsafe block smaller?
— Daniel
Powered by blists - more mailing lists