Re: [PATCH v6 1/3] gpu: nova-core: move the debugfs root into the module data

["Danilo Krummrich" <dakr@kernel.org>]

On Sat Nov 7, 2026 at 7:47 AM CET, Alexandre Courbot wrote:
> On Thu Oct 15, 2026 at 12:45 AM JST, Vladislav Zaharov wrote:
> Another thing: since this is declaring now a proper interface, can you
> move `DebugfsData`, `debugfs_data` and `DEBUGFS_DATA` into a sub-module,
> say `debugfs.rs`?

I think that makes sense.

>>  impl InPlaceModule for NovaCoreModule {
>>      fn init(module: &'static kernel::ThisModule) -> impl PinInit<Self, Error> {
>> -        let dir = debugfs::Dir::new(c"nova-core");
>> -
>> -        // SAFETY: We are the only driver code running during init, so there
>> -        // cannot be any concurrent access to `DEBUGFS_ROOT`.
>> -        unsafe { DEBUGFS_ROOT = Some(dir) };
>> -
>>          try_pin_init!(Self {
>> +            _debugfs <- DebugfsData::new(),
>> +            _: {
>> +                // SAFETY: Nothing reads `DEBUGFS_DATA` before a device is bound, which cannot
>> +                // happen until the driver is registered below. What it points at is dropped only
>> +                // once the driver is unregistered, so it is valid for as long as any device is
>> +                // bound, and nothing reads it outside of that.
>> +                unsafe { DEBUGFS_DATA = &*_debugfs };
>> +            },
>
> Let's move this initialization into `DebugfsData::new`, using
> `pin_chain`:
>
>     /// Returns an initializer that creates the shared debugfs data.
>     ///
>     /// # Safety
>     ///
>     /// The returned initializer must be run exactly once before the driver is registered. The
>     /// initialized [`DebugfsData`] must outlive the driver registration.
>     pub(super) unsafe fn new() -> impl PinInit<Self> {
>         pin_init!(Self {
>             root: debugfs::Dir::new(c"nova-core"),
>         })
>         .pin_chain(|debugfs| {
>             // SAFETY: Per the method's safety block, the driver is not registered yet, meaning that
>             // no device can be probed and thus nothing can access `DEBUGFS_DATA` concurrently.
>             //
>             // INVARIANT: Per the method's safety block, `DEBUGFS_DATA` is written before the driver
>             // is registered and points to a valid instance of `DebugfsData`, which is pinned
>             // and dropped after the driver is unregistered.
>             unsafe { DEBUGFS_DATA = &*debugfs };
>             Ok(())
>         })
>     }
>
> That way, the burden of enforcing the invariants for `DEBUGFS_DATA`
> relies exclusively on `new` (and we made it unsafe to force the caller
> to do that).

Actually, I would leave that part as is.

Eventually we'll have registration data for driver registrations to share module
data. Once we have that, we'd need to change DebugfsData::new() again and move
that part bnack into NovaCoreModule::init().

Besides that, DebugfsData has nothing to do with the current limitation of
publishing module data to driver registrations; the module code seems like the
correct place to deal with this.