Re: [PATCH v2 8/9] drm/tyr: add gpuvas debugfs file

On Fri, Jul 31, 2026 at 01:05:46AM +0800, Alvin Sun wrote:
> Add a gpuvas debugfs file listing all GPU VAs for the Tyr DRM driver.
> Collects VMs into a shared list during firmware init and renders them
> via dump_gpuva_info on read.
> 
> Signed-off-by: Alvin Sun <[email protected]>
> ---
>  drivers/gpu/drm/tyr/debugfs.rs | 65 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tyr/driver.rs  | 16 +++++++++++
>  drivers/gpu/drm/tyr/fw.rs      |  8 ++++++
>  drivers/gpu/drm/tyr/tyr.rs     |  1 +
>  drivers/gpu/drm/tyr/vm.rs      |  5 ++++
>  5 files changed, 95 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tyr/debugfs.rs b/drivers/gpu/drm/tyr/debugfs.rs
> new file mode 100644
> index 0000000000000..d381b1901bd08
> --- /dev/null
> +++ b/drivers/gpu/drm/tyr/debugfs.rs
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +
> +//! Debugfs support for the Tyr DRM driver.
> +
> +use kernel::{
> +    alloc::KVec,
> +    drm,
> +    new_mutex,
> +    prelude::*,
> +    seq_file,
> +    sync::{
> +        Arc,
> +        Mutex, //
> +    }, //
> +};
> +
> +use crate::{
> +    driver::TyrDrmDriver,
> +    vm::Vm, //
> +};
> +
> +/// Registry of VMs for debugfs access.
> +#[pin_data]
> +pub(crate) struct VmRegistry<'drm> {
> +    #[pin]
> +    vms: Mutex<KVec<Arc<Vm<'drm>>>>,
> +}
> +
> +impl<'drm> VmRegistry<'drm> {
> +    pub(crate) fn new() -> impl PinInit<Self> {
> +        pin_init!(Self { vms <- new_mutex!(KVec::new()) })
> +    }
> +
> +    pub(crate) fn register(&self, vm: Arc<Vm<'drm>>) -> Result {
> +        Ok(self.vms.lock().push(vm, GFP_KERNEL)?)
> +    }
> +
> +    fn for_each(&self, mut f: impl FnMut(&Vm<'drm>) -> Result) -> Result {
> +        for vm in self.vms.lock().iter() {
> +            f(vm)?;
> +        }
> +        Ok(())
> +    }
> +}

This code maintains a separate list of all of the vms for access from
debugfs, but I would have expected that this is not necessary. If we
already store the vms inside the driver's private data, then can't we
just access them from the normal storage location?

By having multiple copies of the same information, you risk that they
get out of sync. In this case, you never remove vms from the list even
if the vm stops being used, which seems wrong.

Alice