All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Danilo Krummrich" <dakr@kernel.org>
Cc: linux-pci@vger.kernel.org, ojeda@kernel.org
Subject: Re: [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type
Date: Thu, 13 Aug 2026 17:06:11 +0000	[thread overview]
Message-ID: <20260813170611.A02781F01559@smtp.kernel.org> (raw)
In-Reply-To: <20260813165234.620555-2-dakr@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The lifetime linkage between `IrqVectorRegistration`, `IrqVector`, and `irq::Registration` forces a self-referential struct design, making the API unusable for device drivers.
--

--- Patch [1]: [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type ---
commit d29a976d8fb0d09c0565565666049353a896a317
Author: Danilo Krummrich <dakr@kernel.org>

rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type

This commit converts IrqVectorRegistration to a lifetime-annotated type that
owns the PCI interrupt vector allocation. It modifies IrqVector to hold a
reference to the registration, preventing the allocation from being dropped
while vectors are still in use.

> diff --git a/rust/kernel/pci/irq.rs b/rust/kernel/pci/irq.rs
> index fea484dcf9cf2..daba86505cd2b 100644
> --- a/rust/kernel/pci/irq.rs
> +++ b/rust/kernel/pci/irq.rs
[ ... ]
> @@ -78,6 +75,7 @@ const fn as_raw(self) -> u32 {
>  #[derive(Clone, Copy)]
>  pub struct IrqVector<'a> {
>      dev: &'a Device<Bound>,
> +    reg: &'a IrqVectorRegistration<'a>,
>      index: u32,
>  }
[ ... ]
> +    pub fn index(&self, index: usize) -> Result<IrqVector<'_>> {
> +        if index >= self.len.get() {
> +            return Err(EINVAL);
> +        }
>  
> -        Ok(range)
> +        // SAFETY: index is within bounds of this registration's allocation, and self.dev is
> +        // the device it was allocated from.
> +        Ok(unsafe { IrqVector::new(self.dev, self, index as u32) })
>      }

[Severity: High]
Could this introduce a regression by creating a self-referential struct
requirement for device drivers?

A typical device driver must maintain both the IrqVectorRegistration (to keep
vectors allocated) and the irq::Registration (to keep handlers active) within
its long-lived state struct across the driver lifecycle.

Looking at IrqVectorRegistration::index() in rust/kernel/pci/irq.rs, it returns
an IrqVector tied to the local borrow of self. This lifetime dependency
propagates to irq::Registration.

Storing both objects in the same driver struct requires a self-referential
design, which cannot be safely modeled using the kernel's pin_init macro.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813165234.620555-1-dakr@kernel.org?part=1

  reply	other threads:[~2026-08-13 17:06 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13 16:52 [PATCH v3 0/5] Rework PCI IRQ vector code Danilo Krummrich
2026-08-13 16:52 ` [PATCH v3 1/5] rust: pci: convert IrqVectorRegistration to a lifetime-managed owning type Danilo Krummrich
2026-08-13 17:06   ` sashiko-bot [this message]
2026-08-13 16:52 ` [PATCH v3 2/5] rust: pci: resolve IRQ in index() and embed IrqRequest in IrqVector Danilo Krummrich
2026-08-13 17:00   ` Gary Guo
2026-08-13 17:11   ` sashiko-bot
2026-08-13 16:52 ` [PATCH v3 3/5] rust: pci: remove request_irq() and request_threaded_irq() from Device Danilo Krummrich
2026-08-13 17:05   ` sashiko-bot
2026-08-13 16:52 ` [PATCH v3 4/5] PCI: Add pci_irq_type() to query the allocated interrupt type Danilo Krummrich
2026-08-13 17:04   ` sashiko-bot
2026-08-13 16:52 ` [PATCH v3 5/5] rust: pci: expose " Danilo Krummrich
2026-08-13 17:07   ` sashiko-bot
2026-08-13 17:02 ` [PATCH v3 0/5] Rework PCI IRQ vector code Gary Guo

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=20260813170611.A02781F01559@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dakr@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=ojeda@kernel.org \
    --cc=sashiko-reviews@lists.linux.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.