Re: [PATCH v6 7/7] gpu: nova-core: create GSP-RM logging buffers debugfs entries

["Gary Guo" <gary@garyguo.net>]

On Fri Jan 30, 2026 at 11:36 PM GMT, Timur Tabi wrote:
> On Wed, 2026-01-28 at 20:28 -0600, Timur Tabi wrote:
>> +                    #[allow(static_mut_refs)]
>> +                    // SAFETY: `DEBUGFS_ROOT` is created before driver registration and
>> cleared
>> +                    // after driver unregistration, so no probe() can race with its
>> modification.
>> +                    let log_parent = unsafe { crate::DEBUGFS_ROOT.as_ref() }
>> +                        .expect("DEBUGFS_ROOT not initialized");
>> +
>> +                    log_parent.scope(log_buffers, dev.name(), |logs, dir| {
>> +                        dir.read_binary_file(c_str!("loginit"), &logs.loginit.0);
>> +                        dir.read_binary_file(c_str!("logintr"), &logs.logintr.0);
>> +                        dir.read_binary_file(c_str!("logrm"), &logs.logrm.0);
>
> I think there might be a problem with this code that I don't know how to resolve.
>
> If CONFIG_NOVA_CORE_DEBUGFS=n, then DEBUGFS_ROOT is None, and so the .as_ref() will also be
> none, and the .expect will cause a panic.  We don't want that.
>
> If I remove the .expect(), then log_parent becomes None, but then the .scope() won't compile.
>
> I could wrap the whole thing in #[cfg(CONFIG_NOVA_CORE_DEBUGFS)], but my understanding is that
> the call to .scope() is necessary to ensure that LogBuffers is not dropped at the end of this
> function.
>
> It seems like I'm going to need to do something like this in struct Gsp:
>
> 	#[cfg(CONFIG_NOVA_CORE_DEBUGFS)]
> 	#[pin]
> 	logs: debugfs::Scope<LogBuffers>,
>
> 	#[cfg(not(CONFIG_NOVA_CORE_DEBUGFS))]
> 	logs: LogBuffers,  // Just own them directly, no debugfs
>
> But the design of debugfs is to have it not care if debugfs is disabled.
>
> Any suggestions?

I think the rationale behind current debugfs design is that when it is disabled
in its entirety, then all of the code are compiled out and you're leaved with
ZST, so code don't have to care at all and you'll still have no codegen in the
end.

However, when debugfs is enabled, but CONFIG_NOVA_CORE_DEBUGFS=n, then using
debugfs functionalities would *not* be compiled out (so, for the `Dir::empty()`
patch in the previous iteration, all of the debugging facility would still be
around with CONFIG_DEBUG_FS=y and CONFIG_NOVA_CORE_DEBUGFS=n, which is not
desirable).

The straightforward solution is thus sprinkle `#[cfg(CONFIG_NOVA_CORE_DEBUGFS)]`
everywhere where debugfs is touched, which is non-ideal.

One idea is to create types that look exactly like `Dir` but always ZST and
no-op regardless whether CONFIG_DEBUG_FS is enabled... But that feel a bit..
weird. Matthew, what do you think?

Best,
Gary