Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Thu, 18 Dec 2025 11:10:52 +0100
Message-Id: <DF19F07P95XZ.32ISH5C9QQK75@kernel.org>
Subject: Re: [PATCH v3 6/9] gpu: nova-core: create debugfs root when driver
 loads
Cc: "Gary Guo" <gary@garyguo.net>, <rust-for-linux@vger.kernel.org>, "John
 Hubbard" <jhubbard@nvidia.com>, "Joel Fernandes" <joelagnelf@nvidia.com>,
 "Alexandre Courbot" <acourbot@nvidia.com>, "Lyude Paul" <lyude@redhat.com>,
 <nouveau@lists.freedesktop.org>
To: "Timur Tabi" <ttabi@nvidia.com>
From: "Danilo Krummrich" <dakr@kernel.org>
References: <20251218013910.459045-1-ttabi@nvidia.com>
 <20251218013910.459045-7-ttabi@nvidia.com>
In-Reply-To: <20251218013910.459045-7-ttabi@nvidia.com>

On Thu Dec 18, 2025 at 2:39 AM CET, Timur Tabi wrote:
> Create the 'nova_core' root debugfs entry when the driver loads.
> The Dir entry is part of the NovaCoreModule, so when the module
> unloads, the NovaCoreModule object is dropped, and that drops the
> _debugfs_root, which automatically deletes the debugfs file.
>
> Field order in Rust structs determines drop order - fields are dropped
> in declaration order. By placing _driver before _debugfs_root:
>
> 1. The driver registration is dropped first, which removes all PCI
>    devices and their associated debugfs subdirectories
> 2. Then _debugfs_root is dropped, removing the now-empty nova_core
>    directory
>
> This ordering is critical because debugfs_remove() performs recursive
> removal. If the parent directory were removed first, it would free the
> child dentries, and then the child's drop would attempt to remove an
> already-freed dentry, causing a use-after-free crash.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  drivers/gpu/nova-core/nova_core.rs | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/n=
ova_core.rs
> index 01353be103ca..f17505e5e2b3 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -2,8 +2,7 @@
> =20
>  //! Nova Core GPU Driver
> =20
> -use kernel::{error::Error, pci, prelude::*, InPlaceModule};
> -use pin_init::{PinInit, pinned_drop};
> +use kernel::{debugfs::Dir, error::Error, pci, prelude::*, InPlaceModule}=
;

Again, please use kernel vertical style.

> =20
>  #[macro_use]
>  mod bitfield;
> @@ -24,17 +23,24 @@
> =20
>  pub(crate) const MODULE_NAME: &kernel::str::CStr =3D <LocalModule as ker=
nel::ModuleMetadata>::NAME;
> =20
> -#[pin_data(PinnedDrop)]
> +#[pin_data]
>  struct NovaCoreModule {
> +    // Note: field order matters for drop order. The driver must be drop=
ped before
> +    // the debugfs directory so that device subdirectories are removed f=
irst.

This is not correct, subdirectories have a reference count of their parent
directory. Hence, the drop order does not matter.

>      #[pin]
>      _driver: kernel::driver::Registration<pci::Adapter<driver::NovaCore>=
>,
> +    _debugfs_root: Dir,
>  }
> =20
>  impl InPlaceModule for NovaCoreModule {
>      fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, E=
rror> {
>          pr_info!("NovaCore GPU driver loaded\n");
> +
> +        let debugfs_root =3D Dir::new(kernel::c_str!("nova_core"));

I'd just inline this statement below.

> +
>          try_pin_init!(Self {
>              _driver <- kernel::driver::Registration::new(MODULE_NAME, mo=
dule),
> +            _debugfs_root: debugfs_root,
>          })
>      }
>  }
> --=20
> 2.52.0