Re: [PATCH 1/5] iommu/arm-smmu-v3: Add basic debugfs framework

[Nicolin Chen <nicolinc@nvidia.com>]

On Wed, May 20, 2026 at 02:37:07PM +0800, Qinxin Xia wrote:
> +/**
> + * arm_smmu_debugfs_setup() - Initialize debugfs for SMMU device
> + * @smmu: SMMU device to setup debugfs for
> + * @name: SMMU device name

Nit: "device name" sounds like dev_name(); maybe sysfs node name?

> + * This function creates the basic debugfs directory structure for an SMMU device.
> + *
> + * Return: 0 on success, negative error code on failure

Usually, a debugfs_setup function uses void as a no-fail function.

So, perhaps it should do the same? The caller only spits a warning
print, which could be a part of revert path of this function if it
is really necessary.

> + */
> +int arm_smmu_debugfs_setup(struct arm_smmu_device *smmu, const char *name)
> +{
> +	struct arm_smmu_debugfs *debugfs;
> +	struct dentry *smmu_dir;
> +
> +	/* Create root directory if it doesn't exist */
> +	scoped_guard(mutex, &arm_smmu_debugfs_lock) {
> +		if (!smmu_debugfs_root) {
> +			/* Once created, it will not be removed */
> +			smmu_debugfs_root = debugfs_create_dir("arm_smmu_v3",
> +							       iommu_debugfs_dir);
> +			if (IS_ERR(smmu_debugfs_root)) {
> +				smmu_debugfs_root = NULL;
> +				return -ENOMEM;
> +			}
> +		}
> +	}
> +
> +	/* Allocate debugfs structure */
> +	debugfs = kzalloc_obj(*debugfs);
> +	if (!debugfs)
> +		return -ENOMEM;
> +
> +	/* Create SMMU instance directory */
> +	smmu_dir = debugfs_create_dir(name, smmu_debugfs_root);
> +	if (IS_ERR(smmu_dir)) {
> +		kfree(debugfs);
> +		smmu->debugfs = NULL;

smmu->debugfs is NULL already since it's unset yet.

> +void arm_smmu_debugfs_remove(struct arm_smmu_device *smmu)
> +{
> +	struct arm_smmu_debugfs *debugfs;
> +	struct dentry *smmu_dir;
> +
> +	scoped_guard(mutex, &arm_smmu_debugfs_lock) {
> +		debugfs = smmu->debugfs;

Isn't the lock used to fence smmu_debugfs_root?

smmu->debugfs is per-smmu and freed per-smmu. Where is the race?

Nicolin