Re: [PATCH v3 08/10] rust: pci: add SR-IOV enable and disable tokens

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rust: missing #[inline] for small/binding wrapper abstraction functions
--

commit 4e2162272f9a80159a9c71ad8cb46433c7d6fdae
Author: Zhi Wang <zhiw@nvidia.com>

rust: pci: add SR-IOV enable and disable tokens
    
The commit introduces callback-scoped tokens to manage SR-IOV enable/disable
lifecycles and rollback.

> diff --git a/rust/kernel/pci/iov.rs b/rust/kernel/pci/iov.rs
> index 3411a9101564c..e608b2f1e70c7 100644
> --- a/rust/kernel/pci/iov.rs
> +++ b/rust/kernel/pci/iov.rs
> @@ -47,7 +47,6 @@ pub fn num_vf(&self) -> i32 {
>  impl Device<device::CoreInternal<'_>> {
>      /// Enable the Single Root I/O Virtualization (SR-IOV) capability for this device,
>      /// where `nr_virtfn` is number of Virtual Functions (VF) to enable.
> -    #[expect(dead_code)]
>      pub(crate) fn enable_sriov(&self, nr_virtfn: c_int) -> Result {

[Severity: Low]
This isn't a bug, but should this function have an #[inline] annotation?
The Rust subsystem guidelines state that C binding wrappers should be inlined.

[ ... ]

>      /// Disable the Single Root I/O Virtualization (SR-IOV) capability for this device.
> -    #[expect(dead_code)]
>      pub(crate) fn disable_sriov(&self) {

[Severity: Low]
This isn't a bug, but does this wrapper function also need an #[inline]
annotation since it forwards directly to the C bindings?

[ ... ]

> +impl<'callback> SriovEnable<'callback> {
> +    /// Returns the number of VFs requested for this callback.
> +    pub fn num_vfs(&self) -> u32 {
> +        self.num_vfs
> +    }

[Severity: Low]
This isn't a bug, but could we add an #[inline] annotation to this trivial
getter function?

[ ... ]

> +impl SriovEnabled<'_> {
> +    #[expect(dead_code)]
> +    fn disarm(mut self) -> c_int {
> +        self.pdev = None;
> +        self.num_vfs
> +    }
> +}

[Severity: Low]
This isn't a bug, but would it be appropriate to mark this small
state-mutating function with #[inline]?

[ ... ]

> +impl SriovDisable<'_> {
> +    /// Disables all VFs and waits for their drivers to unbind.
> +    ///
> +    /// The PF resources used by VF drivers must remain available until this method returns.
> +    pub fn disable(self) {
> +        self.pdev.disable_sriov();
> +    }
> +}

[Severity: Low]
This isn't a bug, but since this function directly forwards to disable_sriov(),
should it be annotated with #[inline]?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1790705494.git.zhiw@nvidia.com?part=8