Re: [PATCH] drm: Continue after failure to setup debugfs

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, May 05, 2016 at 10:47:10AM +0100, Chris Wilson wrote:
> If we fail to setup the debugfs we lose debugging convenience, but we
> should still endeavour to enable modesetting so that we can control the
> outputs and enable use of the system to debug the issue. In all
> likelihood if we can not create our debugfs files then there is a larger
> underlying issue that will prevent the module loading, but this should
> get us further towards resilience.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: dri-devel@lists.freedesktop.org

Usually the bug is that someone forgot to clean up debugfs files on module
unload, and there's gunk left behind, and that's causing the failure to
load. Since you can't ever recover from leaking debugfs files in this
fashion you need to reboot anyway to make sure your patches are fixed
properly. So I don't see the value of this all that much ...
-Daniel

> ---
>  drivers/gpu/drm/drm_debugfs.c | 13 +++++++++----
>  drivers/gpu/drm/drm_drv.c     | 19 ++++++++-----------
>  2 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 3bcf8e6a85b3..8f36e014fbd2 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -160,10 +160,8 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  	ret = drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
>  				       minor->debugfs_root, minor);
>  	if (ret) {
> -		debugfs_remove(minor->debugfs_root);
> -		minor->debugfs_root = NULL;
>  		DRM_ERROR("Failed to create core drm debugfs files\n");
> -		return ret;
> +		goto err_root;
>  	}
>  
>  	if (dev->driver->debugfs_init) {
> @@ -171,10 +169,17 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  		if (ret) {
>  			DRM_ERROR("DRM: Driver failed to initialize "
>  				  "/sys/kernel/debug/dri.\n");
> -			return ret;
> +			goto err_core;
>  		}
>  	}
>  	return 0;
> +
> +err_core:
> +	drm_debugfs_remove_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES, minor);
> +err_root:
> +	debugfs_remove(minor->debugfs_root);
> +	minor->debugfs_root = NULL;
> +	return ret;
>  }
>  
>  
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index bff89226a344..18de82e4388a 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -307,11 +307,11 @@ 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) {
> +	ret = -ENOENT;
> +	if (drm_debugfs_root)
> +		ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> +	if (ret)
>  		DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> -		return ret;
> -	}
>  
>  	ret = device_add(minor->kdev);
>  	if (ret)
> @@ -904,17 +904,13 @@ static int __init drm_core_init(void)
>  	}
>  
>  	drm_debugfs_root = debugfs_create_dir("dri", NULL);
> -	if (!drm_debugfs_root) {
> +	if (!drm_debugfs_root)
>  		DRM_ERROR("Cannot create /sys/kernel/debug/dri\n");
> -		ret = -1;
> -		goto err_p3;
> -	}
>  
>  	DRM_INFO("Initialized %s %d.%d.%d %s\n",
>  		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
>  	return 0;
> -err_p3:
> -	drm_sysfs_destroy();
> +
>  err_p2:
>  	unregister_chrdev(DRM_MAJOR, "drm");
>  
> @@ -925,7 +921,8 @@ err_p1:
>  
>  static void __exit drm_core_exit(void)
>  {
> -	debugfs_remove(drm_debugfs_root);
> +	if (drm_debugfs_root)
> +		debugfs_remove(drm_debugfs_root);
>  	drm_sysfs_destroy();
>  
>  	unregister_chrdev(DRM_MAJOR, "drm");
> -- 
> 2.8.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx