diff mbox

drm: Try to acquire modeset lock on panic or sysrq

Message ID 1395750315-22269-1-git-send-email-thierry.reding@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Reding March 25, 2014, 12:25 p.m. UTC
From: Thierry Reding <treding@nvidia.com>

Commit 62ff94a54921 "drm/crtc-helper: remove LOCKING from kerneldoc"
causes drm_helper_crtc_in_use() and drm_helper_encoder_in_use() to
complain loudly during a kernel panic or sysrq processing. This is
caused by nobody acquiring the modeset lock in these code paths.

This patch fixes this by trying to acquire the modeset lock for each
FB helper that's forced to kernel mode. If the lock can't be acquired,
it's likely that somebody else is performing a modeset. However, doing
another modeset concurrently might make things even worse, so the safe
option is to simply bail out in that case.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Daniel Vetter March 25, 2014, 12:37 p.m. UTC | #1
On Tue, Mar 25, 2014 at 01:25:15PM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Commit 62ff94a54921 "drm/crtc-helper: remove LOCKING from kerneldoc"
> causes drm_helper_crtc_in_use() and drm_helper_encoder_in_use() to
> complain loudly during a kernel panic or sysrq processing. This is
> caused by nobody acquiring the modeset lock in these code paths.
> 
> This patch fixes this by trying to acquire the modeset lock for each
> FB helper that's forced to kernel mode. If the lock can't be acquired,
> it's likely that somebody else is performing a modeset. However, doing
> another modeset concurrently might make things even worse, so the safe
> option is to simply bail out in that case.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I guess we should also try to grab the crtc locks, but blocking ongoing
pageflips shouldn't be too harmful. Otherwise I think this should help a
bit in some of the hilarious failures I've seen when kms itself blows up.
-Daniel

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 9580393628a0..afce7cbd5149 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -325,12 +325,21 @@ static bool drm_fb_helper_force_kernel_mode(void)
>  		return false;
>  
>  	list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
> -		if (helper->dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +		struct drm_device *dev = helper->dev;
> +
> +		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +			continue;
> +
> +		if (!mutex_trylock(&dev->mode_config.mutex)) {
> +			error = true;
>  			continue;
> +		}
>  
>  		ret = drm_fb_helper_restore_fbdev_mode(helper);
>  		if (ret)
>  			error = true;
> +
> +		mutex_unlock(&dev->mode_config.mutex);
>  	}
>  	return error;
>  }
> -- 
> 1.9.1
>
David Herrmann March 26, 2014, 12:33 p.m. UTC | #2
Hi

On Tue, Mar 25, 2014 at 1:25 PM, Thierry Reding
<thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Commit 62ff94a54921 "drm/crtc-helper: remove LOCKING from kerneldoc"
> causes drm_helper_crtc_in_use() and drm_helper_encoder_in_use() to
> complain loudly during a kernel panic or sysrq processing. This is
> caused by nobody acquiring the modeset lock in these code paths.
>
> This patch fixes this by trying to acquire the modeset lock for each
> FB helper that's forced to kernel mode. If the lock can't be acquired,
> it's likely that somebody else is performing a modeset. However, doing
> another modeset concurrently might make things even worse, so the safe
> option is to simply bail out in that case.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 9580393628a0..afce7cbd5149 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -325,12 +325,21 @@ static bool drm_fb_helper_force_kernel_mode(void)
>                 return false;
>
>         list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
> -               if (helper->dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +               struct drm_device *dev = helper->dev;
> +
> +               if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> +                       continue;
> +
> +               if (!mutex_trylock(&dev->mode_config.mutex)) {
> +                       error = true;

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

>                         continue;
> +               }
>
>                 ret = drm_fb_helper_restore_fbdev_mode(helper);
>                 if (ret)
>                         error = true;
> +
> +               mutex_unlock(&dev->mode_config.mutex);
>         }
>         return error;
>  }
> --
> 1.9.1
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 9580393628a0..afce7cbd5149 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -325,12 +325,21 @@  static bool drm_fb_helper_force_kernel_mode(void)
 		return false;
 
 	list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
-		if (helper->dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+		struct drm_device *dev = helper->dev;
+
+		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
+			continue;
+
+		if (!mutex_trylock(&dev->mode_config.mutex)) {
+			error = true;
 			continue;
+		}
 
 		ret = drm_fb_helper_restore_fbdev_mode(helper);
 		if (ret)
 			error = true;
+
+		mutex_unlock(&dev->mode_config.mutex);
 	}
 	return error;
 }