Commit cb597bb3 authored by Daniel Vetter's avatar Daniel Vetter

drm: trylock modest locking for fbdev panics

In the fbdev code we want to do trylocks only to avoid deadlocks and
other ugly issues. Thus far we've only grabbed the overall modeset
lock, but that already failed to exclude a pile of potential
concurrent operations. With proper atomic support this will be worse.

So add a trylock mode to the modeset locking code which attempts all
locks only with trylocks, if possible. We need to track this in the
locking functions themselves and can't restrict this to drivers since
driver-private w/w mutexes must be treated the same way.

There's still the issue that other driver private locks aren't handled
here at all, but well can't have everything. With this we will at
least not regress, even once atomic allows lots of concurrent kms
activity.

Aside: We should move the acquire context to stack-based allocation in
the callers to get rid of that awful WARN_ON(kmalloc_failed) control
flow which just blows up when memory is short. But that's material for
separate patches.

v2:
- Fix logic inversion fumble in the fb helper.
- Add proper kerneldoc.
Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
parent 3d30a59b
...@@ -365,11 +365,11 @@ static bool drm_fb_helper_force_kernel_mode(void) ...@@ -365,11 +365,11 @@ static bool drm_fb_helper_force_kernel_mode(void)
if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
continue; continue;
/* NOTE: we use lockless flag below to avoid grabbing other /*
* modeset locks. So just trylock the underlying mutex * NOTE: Use trylock mode to avoid deadlocks and sleeping in
* directly: * panic context.
*/ */
if (!mutex_trylock(&dev->mode_config.mutex)) { if (__drm_modeset_lock_all(dev, true) != 0) {
error = true; error = true;
continue; continue;
} }
...@@ -378,7 +378,7 @@ static bool drm_fb_helper_force_kernel_mode(void) ...@@ -378,7 +378,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
if (ret) if (ret)
error = true; error = true;
mutex_unlock(&dev->mode_config.mutex); drm_modeset_unlock_all(dev);
} }
return error; return error;
} }
......
...@@ -57,26 +57,37 @@ ...@@ -57,26 +57,37 @@
/** /**
* drm_modeset_lock_all - take all modeset locks * __drm_modeset_lock_all - internal helper to grab all modeset locks
* @dev: drm device * @dev: DRM device
* @trylock: trylock mode for atomic contexts
* *
* This function takes all modeset locks, suitable where a more fine-grained * This is a special version of drm_modeset_lock_all() which can also be used in
* scheme isn't (yet) implemented. Locks must be dropped with * atomic contexts. Then @trylock must be set to true.
* drm_modeset_unlock_all. *
* Returns:
* 0 on success or negative error code on failure.
*/ */
void drm_modeset_lock_all(struct drm_device *dev) int __drm_modeset_lock_all(struct drm_device *dev,
bool trylock)
{ {
struct drm_mode_config *config = &dev->mode_config; struct drm_mode_config *config = &dev->mode_config;
struct drm_modeset_acquire_ctx *ctx; struct drm_modeset_acquire_ctx *ctx;
int ret; int ret;
ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); ctx = kzalloc(sizeof(*ctx),
if (WARN_ON(!ctx)) trylock ? GFP_ATOMIC : GFP_KERNEL);
return; if (!ctx)
return -ENOMEM;
if (trylock) {
if (!mutex_trylock(&config->mutex))
return -EBUSY;
} else {
mutex_lock(&config->mutex); mutex_lock(&config->mutex);
}
drm_modeset_acquire_init(ctx, 0); drm_modeset_acquire_init(ctx, 0);
ctx->trylock_only = trylock;
retry: retry:
ret = drm_modeset_lock(&config->connection_mutex, ctx); ret = drm_modeset_lock(&config->connection_mutex, ctx);
...@@ -95,13 +106,29 @@ void drm_modeset_lock_all(struct drm_device *dev) ...@@ -95,13 +106,29 @@ void drm_modeset_lock_all(struct drm_device *dev)
drm_warn_on_modeset_not_all_locked(dev); drm_warn_on_modeset_not_all_locked(dev);
return; return 0;
fail: fail:
if (ret == -EDEADLK) { if (ret == -EDEADLK) {
drm_modeset_backoff(ctx); drm_modeset_backoff(ctx);
goto retry; goto retry;
} }
return ret;
}
EXPORT_SYMBOL(__drm_modeset_lock_all);
/**
* drm_modeset_lock_all - take all modeset locks
* @dev: drm device
*
* This function takes all modeset locks, suitable where a more fine-grained
* scheme isn't (yet) implemented. Locks must be dropped with
* drm_modeset_unlock_all.
*/
void drm_modeset_lock_all(struct drm_device *dev)
{
WARN_ON(__drm_modeset_lock_all(dev, false) != 0);
} }
EXPORT_SYMBOL(drm_modeset_lock_all); EXPORT_SYMBOL(drm_modeset_lock_all);
...@@ -287,7 +314,12 @@ static inline int modeset_lock(struct drm_modeset_lock *lock, ...@@ -287,7 +314,12 @@ static inline int modeset_lock(struct drm_modeset_lock *lock,
WARN_ON(ctx->contended); WARN_ON(ctx->contended);
if (interruptible && slow) { if (ctx->trylock_only) {
if (!ww_mutex_trylock(&lock->mutex))
return -EBUSY;
else
return 0;
} else if (interruptible && slow) {
ret = ww_mutex_lock_slow_interruptible(&lock->mutex, &ctx->ww_ctx); ret = ww_mutex_lock_slow_interruptible(&lock->mutex, &ctx->ww_ctx);
} else if (interruptible) { } else if (interruptible) {
ret = ww_mutex_lock_interruptible(&lock->mutex, &ctx->ww_ctx); ret = ww_mutex_lock_interruptible(&lock->mutex, &ctx->ww_ctx);
......
...@@ -53,6 +53,11 @@ struct drm_modeset_acquire_ctx { ...@@ -53,6 +53,11 @@ struct drm_modeset_acquire_ctx {
* list of held locks (drm_modeset_lock) * list of held locks (drm_modeset_lock)
*/ */
struct list_head locked; struct list_head locked;
/**
* Trylock mode, use only for panic handlers!
*/
bool trylock_only;
}; };
/** /**
...@@ -123,6 +128,7 @@ struct drm_device; ...@@ -123,6 +128,7 @@ struct drm_device;
struct drm_crtc; struct drm_crtc;
void drm_modeset_lock_all(struct drm_device *dev); void drm_modeset_lock_all(struct drm_device *dev);
int __drm_modeset_lock_all(struct drm_device *dev, bool trylock);
void drm_modeset_unlock_all(struct drm_device *dev); void drm_modeset_unlock_all(struct drm_device *dev);
void drm_modeset_lock_crtc(struct drm_crtc *crtc); void drm_modeset_lock_crtc(struct drm_crtc *crtc);
void drm_modeset_unlock_crtc(struct drm_crtc *crtc); void drm_modeset_unlock_crtc(struct drm_crtc *crtc);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment