[PATCH v2 1/2] drm/amdgpu: do not keep debugfs dentry
Christian König
ckoenig.leichtzumerken at gmail.com
Thu Feb 11 12:05:16 UTC 2021
Am 11.02.21 um 12:59 schrieb Nirmoy Das:
> Cleanup unnecessary debugfs dentries and surrounding functions.
>
> v2: remove ttm_debugfs_entries array.
> do not init variables.
>
> 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 | 20 +++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 73 +++++++++------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ras.h | 4 --
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 40 ++++-------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 4 --
> drivers/gpu/drm/amd/pm/inc/amdgpu_smu.h | 3 -
> 7 files changed, 52 insertions(+), 96 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..55db646d0ba4 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;
> @@ -1591,22 +1590,21 @@ DEFINE_SIMPLE_ATTRIBUTE(fops_sclk_set, NULL,
>
> int amdgpu_debugfs_init(struct amdgpu_device *adev)
> {
> + struct dentry *ent;
> int r, i;
>
> - adev->debugfs_preempt =
> - debugfs_create_file("amdgpu_preempt_ib", 0600,
> - adev_to_drm(adev)->primary->debugfs_root, adev,
> - &fops_ib_preempt);
> - if (!(adev->debugfs_preempt)) {
> + ent = debugfs_create_file("amdgpu_preempt_ib", 0600,
> + adev_to_drm(adev)->primary->debugfs_root, adev,
> + &fops_ib_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,
> - adev_to_drm(adev)->primary->debugfs_root, adev,
> - &fops_sclk_set);
> - if (!(adev->smu.debugfs_sclk)) {
> + ent = debugfs_create_file("amdgpu_force_sclk", 0200,
> + adev_to_drm(adev)->primary->debugfs_root, adev,
> + &fops_sclk_set);
> + 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..b504914519ce 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -1137,16 +1137,17 @@ 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)
> +struct dentry *amdgpu_ras_debugfs_create_ctrl_node(struct amdgpu_device *adev)
> {
> struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
> + struct dentry *dir;
> 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,
> - adev, &amdgpu_ras_debugfs_ctrl_ops);
> - debugfs_create_file("ras_eeprom_reset", S_IWUGO | S_IRUGO, con->dir,
> - adev, &amdgpu_ras_debugfs_eeprom_ops);
> + 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, dir, adev,
> + &amdgpu_ras_debugfs_eeprom_ops);
>
> /*
> * After one uncorrectable error happens, usually GPU recovery will
> @@ -1156,24 +1157,24 @@ 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,
> - &con->reboot);
> + debugfs_create_bool("auto_reboot", S_IWUGO | S_IRUGO, dir, &con->reboot);
>
> /*
> * User could set this not to clean up hardware's error count register
> * of RAS IPs during ras recovery.
> */
> - debugfs_create_bool("disable_ras_err_cnt_harvest", 0644,
> - con->dir, &con->disable_ras_err_cnt_harvest);
> + debugfs_create_bool("disable_ras_err_cnt_harvest", 0644, dir,
> + &con->disable_ras_err_cnt_harvest);
> + return dir;
> }
>
> 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,14 +1183,14 @@ 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)
> {
> struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
> + struct dentry *dir;
> struct ras_manager *obj;
> struct ras_fs_if fs_info;
>
> @@ -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);
> + dir = amdgpu_ras_debugfs_create_ctrl_node(adev);
>
> list_for_each_entry(obj, &con->head, node) {
> if (amdgpu_ras_is_supported(adev, obj->head.block) &&
> @@ -1208,34 +1209,11 @@ 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);
> }
> }
> }
>
> -static void amdgpu_ras_debugfs_remove(struct amdgpu_device *adev,
> - struct ras_common_if *head)
> -{
> - struct ras_manager *obj = amdgpu_ras_find_obj(adev, head);
> -
> - if (!obj || !obj->ent)
> - return;
> -
> - obj->ent = NULL;
> - put_obj(obj);
> -}
> -
> -static void amdgpu_ras_debugfs_remove_all(struct amdgpu_device *adev)
> -{
> - struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
> - struct ras_manager *obj, *tmp;
> -
> - list_for_each_entry_safe(obj, tmp, &con->head, node) {
> - amdgpu_ras_debugfs_remove(adev, &obj->head);
> - }
> -
> - con->dir = NULL;
> -}
> /* debugfs end */
>
> /* ras fs */
> @@ -1282,8 +1260,17 @@ static int amdgpu_ras_fs_init(struct amdgpu_device *adev)
>
> static int amdgpu_ras_fs_fini(struct amdgpu_device *adev)
> {
> - if (IS_ENABLED(CONFIG_DEBUG_FS))
> - amdgpu_ras_debugfs_remove_all(adev);
> + struct amdgpu_ras *con = amdgpu_ras_get_context(adev);
> + struct ras_manager *con_obj, *ip_obj, *tmp;
> +
> + if (IS_ENABLED(CONFIG_DEBUG_FS)) {
> + list_for_each_entry_safe(con_obj, tmp, &con->head, node) {
> + ip_obj = amdgpu_ras_find_obj(adev, &con_obj->head);
> + if (ip_obj)
> + put_obj(ip_obj);
> + }
> + }
> +
> amdgpu_ras_sysfs_remove_all(adev);
> return 0;
> }
> 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..b140e6f8209b 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,
> @@ -2530,18 +2530,6 @@ static const struct file_operations amdgpu_ttm_iomem_fops = {
> .llseek = default_llseek
> };
>
> -static const struct {
> - char *name;
> - const struct file_operations *fops;
> - int domain;
> -} ttm_debugfs_entries[] = {
> - { "amdgpu_vram", &amdgpu_ttm_vram_fops, TTM_PL_VRAM },
> -#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
> - { "amdgpu_gtt", &amdgpu_ttm_gtt_fops, TTM_PL_TT },
> -#endif
> - { "amdgpu_iomem", &amdgpu_ttm_iomem_fops, TTM_PL_SYSTEM },
> -};
> -
> #endif
>
> int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
> @@ -2550,22 +2538,20 @@ int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev)
> unsigned count;
>
> struct drm_minor *minor = adev_to_drm(adev)->primary;
> + umode_t mode = S_IFREG | S_IRUGO;
> struct dentry *ent, *root = minor->debugfs_root;
>
> - for (count = 0; count < ARRAY_SIZE(ttm_debugfs_entries); count++) {
> - ent = debugfs_create_file(
> - ttm_debugfs_entries[count].name,
> - S_IFREG | S_IRUGO, root,
> - 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;
> - }
> + debugfs_create_file_size("amdgpu_vram", mode, root, adev,
> + &amdgpu_ttm_vram_fops, adev->gmc.mc_vram_size);
> +#ifdef CONFIG_DRM_AMDGPU_GART_DEBUGFS
> + debugfs_create_file_size("amdgpu_gtt", mode, root, adev,
> + &amdgpu_ttm_gtt_fops, adev->gmc.gart_size);
> +#endif
> +
> + ent = debugfs_create_file("amdgpu_iomem", mode, root, adev,
> + &amdgpu_ttm_iomem_fops);
> + if (IS_ERR(ent))
> + return PTR_ERR(ent);
You should probably drop that. We can print an error, but failing on
debugfs creation isn't helpful at all.
With that fixed the patch series is Reviewed-by: Christian König
<christian.koenig at amd.com>
Thanks,
Christian.
>
> 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;
> --
> 2.30.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
More information about the amd-gfx
mailing list