[Intel-gfx] [PATCH 1/5] dri: cleanup debugfs error handling
Thomas Zimmermann
tzimmermann at suse.de
Fri Oct 8 09:35:33 UTC 2021
Hi
Am 08.10.21 um 11:17 schrieb Nirmoy Das:
> Debugfs API returns encoded error instead of NULL.
> This patch cleanups drm debugfs error handling to
> properly set dri and its minor's root dentry to NULL.
>
> Also do not error out if dri/minor debugfs directory
> creation fails as a debugfs error is not a fatal error.
>
> CC: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> CC: Maxime Ripard <mripard at kernel.org>
> CC: Thomas Zimmermann <tzimmermann at suse.de>
> CC: David Airlie <airlied at linux.ie>
> CC: Daniel Vetter <daniel at ffwll.ch>
> Signed-off-by: Nirmoy Das <nirmoy.das at amd.com>
> ---
> drivers/gpu/drm/drm_debugfs.c | 25 +++++++++++++++++++++++--
> drivers/gpu/drm/drm_drv.c | 16 ++++++++++------
> drivers/gpu/drm/drm_internal.h | 7 +++----
> 3 files changed, 36 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index b0a826489488..af275a0c09b4 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -180,6 +180,9 @@ void drm_debugfs_create_files(const struct drm_info_list *files, int count,
> struct drm_info_node *tmp;
> int i;
>
> + if (!minor->debugfs_root)
> + return;
> +
> for (i = 0; i < count; i++) {
> u32 features = files[i].driver_features;
>
> @@ -203,7 +206,7 @@ void drm_debugfs_create_files(const struct drm_info_list *files, int count,
> }
> EXPORT_SYMBOL(drm_debugfs_create_files);
>
> -int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> +void drm_debugfs_init(struct drm_minor *minor, int minor_id,
> struct dentry *root)
> {
> struct drm_device *dev = minor->dev;
> @@ -212,8 +215,16 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> INIT_LIST_HEAD(&minor->debugfs_list);
> mutex_init(&minor->debugfs_lock);
> sprintf(name, "%d", minor_id);
> +
> + if (!root)
> + goto error;
> +
> minor->debugfs_root = debugfs_create_dir(name, root);
>
> + if (IS_ERR(minor->debugfs_root))
> + goto error;
> +
> +
> drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
> minor->debugfs_root, minor);
>
> @@ -230,7 +241,11 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> if (dev->driver->debugfs_init)
> dev->driver->debugfs_init(minor);
>
> - return 0;
> + return;
> +
> +error:
> + minor->debugfs_root = NULL;
> + return;
> }
>
>
> @@ -241,6 +256,9 @@ int drm_debugfs_remove_files(const struct drm_info_list *files, int count,
> struct drm_info_node *tmp;
> int i;
>
> + if (!minor->debugfs_root)
> + return 0;
> +
> mutex_lock(&minor->debugfs_lock);
> for (i = 0; i < count; i++) {
> list_for_each_safe(pos, q, &minor->debugfs_list) {
> @@ -261,6 +279,9 @@ static void drm_debugfs_remove_all_files(struct drm_minor *minor)
> {
> struct drm_info_node *node, *tmp;
>
> + if (!minor->debugfs_root)
> + return;
> +
> mutex_lock(&minor->debugfs_lock);
> list_for_each_entry_safe(node, tmp, &minor->debugfs_list, list) {
> debugfs_remove(node->dent);
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7a5097467ba5..fa57ec2d49bf 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -160,11 +160,7 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> if (!minor)
> return 0;
>
> - ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> - if (ret) {
> - DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
Rather than deleting the error message, return an error code from
drm_debugfs_init() and print it here. I'd change DRM_ERROR() to
drm_dbg_core(NULL, ...).
> - goto err_debugfs;
> - }
> + drm_debugfs_init(minor, minor->index, drm_debugfs_root);
>
> ret = device_add(minor->kdev);
> if (ret)
> @@ -1050,7 +1046,15 @@ static int __init drm_core_init(void)
> goto error;
> }
>
> - drm_debugfs_root = debugfs_create_dir("dri", NULL);
> + if (!debugfs_initialized()) {
> + drm_debugfs_root = NULL;
> + } else {
> + drm_debugfs_root = debugfs_create_dir("dri", NULL);
> + if (IS_ERR(drm_debugfs_root)) {
> + DRM_WARN("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
This should also print the error code. I'd also change the call to
drm_dbg_core(). The message should say 'failed to create', so it's
differnt from the other one.
Best regards
Thomas
> + drm_debugfs_root = NULL;
> + }
> + }
>
> ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
> if (ret < 0)
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index 17f3548c8ed2..e27a40166178 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -182,8 +182,8 @@ int drm_gem_dumb_destroy(struct drm_file *file, struct drm_device *dev,
>
> /* drm_debugfs.c drm_debugfs_crc.c */
> #if defined(CONFIG_DEBUG_FS)
> -int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> - struct dentry *root);
> +void drm_debugfs_init(struct drm_minor *minor, int minor_id,
> + struct dentry *root);
> void drm_debugfs_cleanup(struct drm_minor *minor);
> void drm_debugfs_connector_add(struct drm_connector *connector);
> void drm_debugfs_connector_remove(struct drm_connector *connector);
> @@ -191,10 +191,9 @@ void drm_debugfs_crtc_add(struct drm_crtc *crtc);
> void drm_debugfs_crtc_remove(struct drm_crtc *crtc);
> void drm_debugfs_crtc_crc_add(struct drm_crtc *crtc);
> #else
> -static inline int drm_debugfs_init(struct drm_minor *minor, int minor_id,
> +static inline void drm_debugfs_init(struct drm_minor *minor, int minor_id,
> struct dentry *root)
> {
> - return 0;
> }
>
> static inline void drm_debugfs_cleanup(struct drm_minor *minor)
> --
> 2.32.0
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20211008/d28135f8/attachment.sig>
More information about the Intel-gfx
mailing list