[PATCH 1/2] drm/amdgpu: do not keep debugfs dentry

Christian König ckoenig.leichtzumerken at gmail.com
Wed Feb 10 18:22:28 UTC 2021


Am 10.02.21 um 17:06 schrieb Nirmoy Das:
> Cleanup unwanted  debugfs dentries.

Maybe write unnecessary instead of unwanted.

>
> Signed-off-by: Nirmoy Das <nirmoy.das at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h         |  4 ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 12 +++-----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c     | 34 ++++++++++-----------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h     |  4 ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  4 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  4 ---
>   drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h     |  3 --
>   7 files changed, 23 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index e0c797a5f739..e3d4d2dcb3a0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -797,10 +797,6 @@ struct amdgpu_device {
>   	struct amdgpu_i2c_chan		*i2c_bus[AMDGPU_MAX_I2C_BUS];
>   	struct amdgpu_debugfs		debugfs[AMDGPU_DEBUGFS_MAX_COMPONENTS];
>   	unsigned			debugfs_count;
> -#if defined(CONFIG_DEBUG_FS)
> -	struct dentry                   *debugfs_preempt;
> -	struct dentry			*debugfs_regs[AMDGPU_DEBUGFS_MAX_COMPONENTS];
> -#endif
>   	struct amdgpu_atif		*atif;
>   	struct amdgpu_atcs		atcs;
>   	struct mutex			srbm_mutex;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 4c38c5771cbc..874522217b7c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -1228,7 +1228,6 @@ int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
>   					  adev, debugfs_regs[i]);
>   		if (!i && !IS_ERR_OR_NULL(ent))
>   			i_size_write(ent->d_inode, adev->rmmio_size);
> -		adev->debugfs_regs[i] = ent;
>   	}
>   
>   	return 0;
> @@ -1592,21 +1591,20 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_sclk_set, NULL,
>   int amdgpu_debugfs_init(struct amdgpu_device *adev)
>   {
>   	int r, i;
> +	struct dentry *ent = NULL;

Please don't initialize the variables and sort them so that r and i are 
last.

>   
> -	adev->debugfs_preempt =
> -		debugfs_create_file("amdgpu_preempt_ib", 0600,
> +	ent = debugfs_create_file("amdgpu_preempt_ib", 0600,
>   				    adev_to_drm(adev)->primary->debugfs_root, adev,
>   				    &fops_ib_preempt);
> -	if (!(adev->debugfs_preempt)) {
> +	if (!ent) {
>   		DRM_ERROR("unable to create amdgpu_preempt_ib debugsfs file\n");
>   		return -EIO;
>   	}
>   
> -	adev->smu.debugfs_sclk =
> -		debugfs_create_file("amdgpu_force_sclk", 0200,
> +	ent = debugfs_create_file("amdgpu_force_sclk", 0200,
>   				    adev_to_drm(adev)->primary->debugfs_root, adev,
>   				    &fops_sclk_set);
> -	if (!(adev->smu.debugfs_sclk)) {
> +	if (!ent) {
>   		DRM_ERROR("unable to create amdgpu_set_sclk debugsfs file\n");
>   		return -EIO;
>   	}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 1fb2a91ad30a..1efdfb9b5506 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -1137,15 +1137,16 @@ static int amdgpu_ras_sysfs_remove_all(struct amdgpu_device *adev)
>    *
>    */
>   /* debugfs begin */
> -static void amdgpu_ras_debugfs_create_ctrl_node(struct amdgpu_device *adev)
> +static void amdgpu_ras_debugfs_create_ctrl_node(struct amdgpu_device *adev,
> +						struct dentry *dir)
>   {
>   	struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
>   	struct drm_minor *minor = adev_to_drm(adev)->primary;
>   
> -	con->dir = debugfs_create_dir(RAS_FS_NAME, minor->debugfs_root);
> -	debugfs_create_file("ras_ctrl", S_IWUGO | S_IRUGO, con->dir,
> +	dir = debugfs_create_dir(RAS_FS_NAME, minor->debugfs_root);
> +	debugfs_create_file("ras_ctrl", S_IWUGO | S_IRUGO, dir,
>   				adev, &amdgpu_ras_debugfs_ctrl_ops);
> -	debugfs_create_file("ras_eeprom_reset", S_IWUGO | S_IRUGO, con->dir,
> +	debugfs_create_file("ras_eeprom_reset", S_IWUGO | S_IRUGO, dir,
>   				adev, &amdgpu_ras_debugfs_eeprom_ops);
>   
>   	/*
> @@ -1156,7 +1157,7 @@ static void amdgpu_ras_debugfs_create_ctrl_node(struct amdgpu_device *adev)
>   	 * ERREVENT_ATHUB_INTERRUPT generated. Normal GPU recovery routine
>   	 * will never be called.
>   	 */
> -	debugfs_create_bool("auto_reboot", S_IWUGO | S_IRUGO, con->dir,
> +	debugfs_create_bool("auto_reboot", S_IWUGO | S_IRUGO, dir,
>   				&con->reboot);
>   
>   	/*
> @@ -1164,16 +1165,16 @@ static void amdgpu_ras_debugfs_create_ctrl_node(struct amdgpu_device *adev)
>   	 * of RAS IPs during ras recovery.
>   	 */
>   	debugfs_create_bool("disable_ras_err_cnt_harvest", 0644,
> -			con->dir, &con->disable_ras_err_cnt_harvest);
> +			dir, &con->disable_ras_err_cnt_harvest);
>   }
>   
>   static void amdgpu_ras_debugfs_create(struct amdgpu_device *adev,
> -		struct ras_fs_if *head)
> +				      struct ras_fs_if *head,
> +				      struct dentry *dir)
>   {
> -	struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
>   	struct ras_manager *obj = amdgpu_ras_find_obj(adev, &head->head);
>   
> -	if (!obj || obj->ent)
> +	if (!obj || !dir)
>   		return;
>   
>   	get_obj(obj);
> @@ -1182,9 +1183,8 @@ static void amdgpu_ras_debugfs_create(struct amdgpu_device *adev,
>   			head->debugfs_name,
>   			sizeof(obj->fs_data.debugfs_name));
>   
> -	obj->ent = debugfs_create_file(obj->fs_data.debugfs_name,
> -				       S_IWUGO | S_IRUGO, con->dir, obj,
> -				       &amdgpu_ras_debugfs_ops);
> +	debugfs_create_file(obj->fs_data.debugfs_name, S_IWUGO | S_IRUGO, dir,
> +			    obj, &amdgpu_ras_debugfs_ops);
>   }
>   
>   void amdgpu_ras_debugfs_create_all(struct amdgpu_device *adev)
> @@ -1192,6 +1192,7 @@ void amdgpu_ras_debugfs_create_all(struct amdgpu_device *adev)
>   	struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
>   	struct ras_manager *obj;
>   	struct ras_fs_if fs_info;
> +	struct dentry *dir = NULL;
>   
>   	/*
>   	 * it won't be called in resume path, no need to check
> @@ -1200,7 +1201,7 @@ void amdgpu_ras_debugfs_create_all(struct amdgpu_device *adev)
>   	if (!IS_ENABLED(CONFIG_DEBUG_FS) || !con)
>   		return;
>   
> -	amdgpu_ras_debugfs_create_ctrl_node(adev);
> +	amdgpu_ras_debugfs_create_ctrl_node(adev, dir);

That won't work. You need to return the created directory here.

And please stop initializing variables if it isn't explicitly needed, 
the compiler would have pointed out that there is something wrong here 
without that.

>   
>   	list_for_each_entry(obj, &con->head, node) {
>   		if (amdgpu_ras_is_supported(adev, obj->head.block) &&
> @@ -1208,7 +1209,7 @@ void amdgpu_ras_debugfs_create_all(struct amdgpu_device *adev)
>   			sprintf(fs_info.debugfs_name, "%s_err_inject",
>   					ras_block_str(obj->head.block));
>   			fs_info.head = obj->head;
> -			amdgpu_ras_debugfs_create(adev, &fs_info);
> +			amdgpu_ras_debugfs_create(adev, &fs_info, dir);
>   		}
>   	}
>   }
> @@ -1218,10 +1219,9 @@ static void amdgpu_ras_debugfs_remove(struct amdgpu_device *adev,
>   {
>   	struct ras_manager *obj = amdgpu_ras_find_obj(adev, head);
>   
> -	if (!obj || !obj->ent)
> +	if (!obj)
>   		return;
>   
> -	obj->ent = NULL;
>   	put_obj(obj);
>   }

Is that function really doing anything any more? I don't think so.

Regards,
Christian.

>   
> @@ -1233,8 +1233,6 @@ static void amdgpu_ras_debugfs_remove_all(struct amdgpu_device *adev)
>   	list_for_each_entry_safe(obj, tmp, &con->head, node) {
>   		amdgpu_ras_debugfs_remove(adev, &obj->head);
>   	}
> -
> -	con->dir = NULL;
>   }
>   /* debugfs end */
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> index 762f5e46c007..aed0716efa5a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h
> @@ -318,8 +318,6 @@ struct amdgpu_ras {
>   	uint32_t supported;
>   	uint32_t features;
>   	struct list_head head;
> -	/* debugfs */
> -	struct dentry *dir;
>   	/* sysfs */
>   	struct device_attribute features_attr;
>   	struct bin_attribute badpages_attr;
> @@ -395,8 +393,6 @@ struct ras_manager {
>   	struct list_head node;
>   	/* the device */
>   	struct amdgpu_device *adev;
> -	/* debugfs */
> -	struct dentry *ent;
>   	/* sysfs */
>   	struct device_attribute sysfs_attr;
>   	int attr_inuse;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index aaad9e304ad9..f4d7cf60c262 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1788,7 +1788,7 @@ static void amdgpu_ttm_training_data_block_init(struct amdgpu_device *adev)
>   		(adev->gmc.mc_vram_size - GDDR6_MEM_TRAINING_OFFSET);
>   	ctx->train_data_size =
>   		GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES;
> -	
> +
>   	DRM_DEBUG("train_data_size:%llx,p2c_train_data_offset:%llx,c2p_train_data_offset:%llx.\n",
>   			ctx->train_data_size,
>   			ctx->p2c_train_data_offset,
> @@ -2560,11 +2560,11 @@ int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
>   				ttm_debugfs_entries[count].fops);
>   		if (IS_ERR(ent))
>   			return PTR_ERR(ent);
> +
>   		if (ttm_debugfs_entries[count].domain == TTM_PL_VRAM)
>   			i_size_write(ent->d_inode, adev->gmc.mc_vram_size);
>   		else if (ttm_debugfs_entries[count].domain == TTM_PL_TT)
>   			i_size_write(ent->d_inode, adev->gmc.gart_size);
> -		adev->mman.debugfs_entries[count] = ent;
>   	}
>   
>   	count = ARRAY_SIZE(amdgpu_ttm_debugfs_list);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index 6c142455fc66..4df4cf2fd4dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -68,10 +68,6 @@ struct amdgpu_mman {
>   	bool				initialized;
>   	void __iomem			*aper_base_kaddr;
>   
> -#if defined(CONFIG_DEBUG_FS)
> -	struct dentry			*debugfs_entries[8];
> -#endif
> -
>   	/* buffer handling */
>   	const struct amdgpu_buffer_funcs	*buffer_funcs;
>   	struct amdgpu_ring			*buffer_funcs_ring;
> diff --git a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> index 10b0624ade65..83147b7d836e 100644
> --- a/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h
> @@ -439,9 +439,6 @@ struct smu_context
>   	struct smu_baco_context		smu_baco;
>   	struct smu_temperature_range	thermal_range;
>   	void *od_settings;
> -#if defined(CONFIG_DEBUG_FS)
> -	struct dentry                   *debugfs_sclk;
> -#endif
>   
>   	struct smu_umd_pstate_table	pstate_table;
>   	uint32_t pstate_sclk;



More information about the amd-gfx mailing list