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

[Alexandre Courbot]

On Sat Nov 7, 2026 at 10:14 PM JST, Danilo Krummrich wrote:
> 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.

Makes sense, let's reduce the churn indeed. In this case I guess
`DEBUGFS_DATA` will need to be made `pub(super)` so this module can
initialize it.