Re: [PATCH v7 3/4] gpu: nova-core: gsp: retain the GSP-RM log buffers after unbind

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

On Fri Nov 13, 2026 at 8:53 AM CET, Vladislav Zaharov wrote:
> +    /// Returns the copies of the GSP-RM log buffers of GPUs that are no longer bound.
> +    pub(crate) fn retained_logs(&self) -> &Mutex<RetainedLogs> {
> +        &self.retained_logs
> +    }

NIT: Should be #[inline].

> +    /// Exposes copies of the buffers in their place, if `gsp_keep_logs` is set.
> +    fn retain(&self) -> Result {
> +        if !crate::module_parameters::gsp_keep_logs.value() || !cfg!(CONFIG_DEBUG_FS) {
> +            return Ok(());
> +        }

This misses debugfs being disabled through the kernel command line with
debugfs=off.

Usually, drivers should treat debugfs availability as opaque. The Rust
abstraction keeps its backing data alive regardless of whether the entries were
created.

Here, the copies serve no purpose without debugfs, so checking availability
would have a concrete benefit. However, I don't think this case warrants
exposing debugfs::initialized(), which could be abused easily and encourage
unnecessary checks elsewhere.

Let's check only gsp_keep_logs.

> +
> +        let logs = RetainedLogBuffers {
> +            name: CString::try_from_fmt(fmt!("{}", self.dev.name()))?,
> +            loginit: self.loginit.snapshot()?,
> +            logintr: self.logintr.snapshot()?,
> +            logrm: self.logrm.snapshot()?,
> +        };
> +
> +        if logs.loginit.is_empty() && logs.logintr.is_empty() && logs.logrm.is_empty() {
> +            return Ok(());
> +        }
> +
> +        // Allocate before an earlier copy is dropped, so that a failure leaves that copy in place.
> +        let scope = KBox::<debugfs::Scope<RetainedLogBuffers>>::new_uninit(GFP_KERNEL)?;
> +        let data = NovaCoreModule::debugfs_data(self.dev);
> +        let root = data.root();
> +        let mut retained = data.retained_logs().lock();
> +        retained.0.reserve(1, GFP_KERNEL)?;

This shouldn't be necessary.

> +
> +        retained.remove(self.dev.name());
> +
> +        let scope = scope.write_pin_init(root.scope(logs, self.dev.name(), |logs, dir| {
> +            if !logs.loginit.is_empty() {
> +                dir.read_binary_file(c"loginit", &logs.loginit);
> +            }
> +            if !logs.logintr.is_empty() {
> +                dir.read_binary_file(c"logintr", &logs.logintr);
> +            }
> +            if !logs.logrm.is_empty() {
> +                dir.read_binary_file(c"logrm", &logs.logrm);
> +            }
> +        }))?;
> +
> +        retained.0.push(scope, GFP_KERNEL)?;

Please add a forwarding method for this.