From: "Ewan Chorynski" <ewan.chorynski@ik.me>
To: "Danilo Krummrich" <dakr@kernel.org>,
"Maurice Hieronymus" <mhi@mailbox.org>
Cc: "Bjorn Helgaas" <bhelgaas@google.com>,
"Krzysztof Wilczyński" <kwilczynski@kernel.org>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Boqun Feng" <boqun@kernel.org>, "Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <lossin@kernel.org>,
"Andreas Hindborg" <a.hindborg@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"Trevor Gross" <tmgross@umich.edu>,
"Daniel Almeida" <daniel.almeida@collabora.com>,
"Tamir Duberstein" <tamird@kernel.org>,
"Alexandre Courbot" <acourbot@nvidia.com>,
"Onur Özkan" <work@onurozkan.dev>,
"Lyude Paul" <lyude@redhat.com>,
linux-pci@vger.kernel.org, rust-for-linux@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/4] rust: samples: add EDU PCI driver sample
Date: Fri, 31 Jul 2026 16:25:51 +0200 [thread overview]
Message-ID: <DKCTQT2XK3O9.33AVSQ4182B9N@ik.me> (raw)
In-Reply-To: <DJP8UPM9XT95.102LJRRRY4P2S@kernel.org>
Hi,
On Fri Jul 3, 2026 at 11:12 PM CEST, Danilo Krummrich wrote:
> On Sun Jun 21, 2026 at 2:27 PM CEST, Danilo Krummrich wrote:
>> I'm going to send a patch to make the irq::Registration compatible with the
>> device driver lifetime rework soon. Please rebase onto that once it's sent, so
>> this sample can land as idiomatic as possible.
>
> Please find the patch in [1]; purely mechanical conversion of the EDU driver for
> testing purposes in [2].
>
> [1] https://lore.kernel.org/driver-core/20260703210936.1128698-1-dakr@kernel.org/
>
> [2]
>
> diff --git a/samples/rust/rust_driver_edu.rs b/samples/rust/rust_driver_edu.rs
> index 5f4efd514032..301b780773cb 100644
> --- a/samples/rust/rust_driver_edu.rs
> +++ b/samples/rust/rust_driver_edu.rs
> @@ -6,7 +6,6 @@
>
<snip>
> @@ -275,76 +279,61 @@ fn probe<'bound>(
> .alloc_irq_vectors(1, 1, IrqTypes::default().with(pci::IrqType::Msi))
> .inspect_err(|e| dev_err!(pdev, "alloc_irq_vectors failed: {:?}\n", e))?;
>
> - // State shared with the IRQ handler (the BAR and the completion the
> - // handler signals) lives in an `Arc<EduDriverData>`. `EduDriverData`
> - // itself implements `irq::Handler`, and the registration takes an
> - // `Arc<T>` via the `impl Handler for Arc<T>` blanket impl. This keeps
> - // the handler's state out of `EduDriver` and avoids a self-reference.
> - let data = Arc::pin_init(
> - try_pin_init!(EduDriverData {
> - bar <- pdev
> - .iomap_region_sized(0, c"rust_driver_edu")
> - .and_then(|bar| bar.into_devres()),
> - irq_test_completion <- Completion::new(),
> - irq_dma_completion <- Completion::new(),
> - dma: ca,
> - }),
> - GFP_KERNEL,
> - )?;
> -
> - let req = irq::Registration::new(
> - (*irq.start()).try_into()?,
> - Flags::TRIGGER_NONE,
> - c"rust_edu_irq",
> - Ok(data.clone()),
> - );
> + let bar = pdev.iomap_region_sized::<{ regs::END }>(0, c"rust_driver_edu")?;
>
> - // Ordering matters: the handler is registered (`irq_handler <- req`)
> - // *before* the `_:` block runs the self-tests, one of which raises an
> - // interrupt and waits for the handler. Raising before the handler is
> - // registered would hang (the completion is never signalled).
> - Ok(try_pin_init!(Self {
> + // SAFETY: The Registration is not leaked.
This comment uses the knowledge that the initializer returned by probe
won't create and leak the registration. Shouldn't the probe function be
unsafe to formalize this guarantee ?
> + let req = unsafe {
> + irq::Registration::new(
> + (*irq.start()).try_into()?,
> + Flags::TRIGGER_NONE,
> + c"rust_edu_irq",
> + try_pin_init!(IrqHandler {
> + pdev,
> + bar,
> + irq_test_completion <- Completion::new(),
> + irq_dma_completion <- Completion::new(),
> + dma: ca,
> + }? Error),
> + )
> + };
> +
> + Ok(try_pin_init!(EduDriverData {
> irq_handler <- req,
> - // Side-effect block: run the staged self-tests against the mapped
> - // BAR now that the handler is live. A failure here aborts probe.
> + // Ordering matters: the handler is registered (`irq_handler <- req`)
> + // *before* the `_:` block runs the self-tests, one of which raises an
> + // interrupt and waits for the handler. Raising before the handler is
> + // registered would hang (the completion is never signalled).
> _: {
> - let bar = data.bar.access(pdev.as_ref())?;
> - EduDriver::init(pdev, bar, &data)?;
> + let handler = irq_handler.handler();
> + EduDriver::init(pdev, &handler.bar, handler)?;
> dev_info!(
> pdev,
> "rust_driver_edu successfully initialized\n",
> );
> },
> - data,
> - pdev: pdev.into()
> + pdev,
> }))
> })
> }
> }
Regards,
Ewan
prev parent reply other threads:[~2026-07-31 14:25 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-20 8:45 [PATCH v2 0/4] rust: samples: add an EDU PCI driver sample (MMIO + IRQ + DMA) Maurice Hieronymus
2026-06-20 8:45 ` [PATCH v2 1/4] rust: pci: make Vendor::from_raw() public Maurice Hieronymus
2026-06-20 9:48 ` Onur Özkan
2026-06-20 8:45 ` [PATCH v2 2/4] rust: pci: add managed Device::enable_device() Maurice Hieronymus
2026-06-20 9:54 ` Onur Özkan
2026-06-20 22:19 ` Maurice Hieronymus
2026-06-21 12:02 ` Danilo Krummrich
2026-06-20 8:45 ` [PATCH v2 3/4] rust: completion: add complete() Maurice Hieronymus
2026-06-20 9:48 ` Onur Özkan
2026-07-02 14:11 ` Maurice Hieronymus
2026-07-02 14:42 ` Gary Guo
2026-07-02 14:43 ` Gary Guo
2026-06-20 8:45 ` [PATCH v2 4/4] rust: samples: add EDU PCI driver sample Maurice Hieronymus
2026-06-20 9:45 ` Onur Özkan
2026-06-21 12:27 ` Danilo Krummrich
2026-07-02 15:24 ` Maurice Hieronymus
2026-07-02 15:40 ` Danilo Krummrich
2026-07-03 21:12 ` Danilo Krummrich
2026-07-31 14:25 ` Ewan Chorynski [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DKCTQT2XK3O9.33AVSQ4182B9N@ik.me \
--to=ewan.chorynski@ik.me \
--cc=a.hindborg@kernel.org \
--cc=acourbot@nvidia.com \
--cc=aliceryhl@google.com \
--cc=bhelgaas@google.com \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun@kernel.org \
--cc=dakr@kernel.org \
--cc=daniel.almeida@collabora.com \
--cc=gary@garyguo.net \
--cc=kwilczynski@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lossin@kernel.org \
--cc=lyude@redhat.com \
--cc=mhi@mailbox.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=tamird@kernel.org \
--cc=tmgross@umich.edu \
--cc=work@onurozkan.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®