From patchwork Sun Jul 27 21:41:47 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Vetter X-Patchwork-Id: 4631581 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 9FFCF9F3B4 for ; Sun, 27 Jul 2014 21:42:20 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B432B2012B for ; Sun, 27 Jul 2014 21:42:19 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 7F1ED20120 for ; Sun, 27 Jul 2014 21:42:18 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BF6D26E39A; Sun, 27 Jul 2014 14:42:17 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wg0-f42.google.com (mail-wg0-f42.google.com [74.125.82.42]) by gabe.freedesktop.org (Postfix) with ESMTP id 87E216E374 for ; Sun, 27 Jul 2014 14:42:08 -0700 (PDT) Received: by mail-wg0-f42.google.com with SMTP id l18so6404921wgh.13 for ; Sun, 27 Jul 2014 14:42:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=nJwc3ZC2KGsxgQ0NTGgONpEeYka4yVo7+MXOpX66/tk=; b=V0Guf2MKLiuq/JZ8OVK3E5EUiXCyjead6fNaOmOi5JQEcJkYVDdoLbXCGyEOr7wr5m Ax3cROXTjX5IW3HslpSHZP8Wbre462bRtn3ByYfH3p409l+pofbszlNIYBgovUkkIvE4 tbhc2AU4GBsnYBGozB/hltiWysmvYcDOMGUYA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=nJwc3ZC2KGsxgQ0NTGgONpEeYka4yVo7+MXOpX66/tk=; b=cqErDV07k0A89b8kzJ/3xdFySnjM366ggSPOVcWkWvLKxp5Of5LDyG/tOAxNG42hmm 41Pwk3IJnCwD7hXviVeI0sp+rET7ALiYI3NlOlRG5PFmjxQWUOy0f2aBP0gyv3nh3vma MxYV7ifQq/DOHN8cFTlGWLvm0ILh6gmQxL4gkalLOir7gIBrHqaZWuM519iqeRbi7dvZ UKwC6qbf5VILZquJBaF1dfNJQXR9uMmf2LAbJfXLH2NftAbOCbefKAAiMplPEHUc0cr1 zu39ReGtlIW15MI2yEnmRVCFVvdHdVti1AKaSYp6zDHS2bMM0/cfhyUexYWr0czQe12X LgEA== X-Gm-Message-State: ALoCoQkhGwgrPGZB0oYzuHaU3BiVG6+VczmhU2fx0/vWf8Bgnez5szA1OfTLjy5o0tMkmmSjvqWu X-Received: by 10.194.7.67 with SMTP id h3mr41078627wja.111.1406497327840; Sun, 27 Jul 2014 14:42:07 -0700 (PDT) Received: from phenom.ffwll.local (84-73-67-144.dclient.hispeed.ch. [84.73.67.144]) by mx.google.com with ESMTPSA id di7sm44343171wjb.34.2014.07.27.14.42.06 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sun, 27 Jul 2014 14:42:07 -0700 (PDT) From: Daniel Vetter To: DRI Development Subject: [PATCH 18/19] drm: trylock modest locking for fbdev panics Date: Sun, 27 Jul 2014 23:41:47 +0200 Message-Id: <1406497308-30733-19-git-send-email-daniel.vetter@ffwll.ch> X-Mailer: git-send-email 2.0.1 In-Reply-To: <1406497308-30733-1-git-send-email-daniel.vetter@ffwll.ch> References: <1406497308-30733-1-git-send-email-daniel.vetter@ffwll.ch> Cc: Daniel Vetter X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,RP_MATCHES_RCVD,T_DKIM_INVALID,UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_fb_helper.c | 10 ++++---- drivers/gpu/drm/drm_modeset_lock.c | 51 +++++++++++++++++++++++++++----------- include/drm/drm_modeset_lock.h | 6 +++++ 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 3144db9dc0f1..66a438ab4e7f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -419,11 +419,11 @@ static bool drm_fb_helper_force_kernel_mode(void) if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) continue; - /* NOTE: we use lockless flag below to avoid grabbing other - * modeset locks. So just trylock the underlying mutex - * directly: + /* + * NOTE: Use trylock mode to avoid deadlocks and sleeping in + * panic context. */ - if (!mutex_trylock(&dev->mode_config.mutex)) { + if (!__drm_modeset_lock_all(dev, true)) { error = true; continue; } @@ -432,7 +432,7 @@ static bool drm_fb_helper_force_kernel_mode(void) if (ret) error = true; - mutex_unlock(&dev->mode_config.mutex); + drm_modeset_unlock_all(dev); } return error; } diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c index 412b396af740..42ba9ca699ad 100644 --- a/drivers/gpu/drm/drm_modeset_lock.c +++ b/drivers/gpu/drm/drm_modeset_lock.c @@ -56,27 +56,27 @@ */ -/** - * 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) +int __drm_modeset_lock_all(struct drm_device *dev, + bool trylock) { struct drm_mode_config *config = &dev->mode_config; struct drm_modeset_acquire_ctx *ctx; int ret; - ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); - if (WARN_ON(!ctx)) - return; + ctx = kzalloc(sizeof(*ctx), + trylock ? GFP_ATOMIC : GFP_KERNEL); + if (!ctx) + return -ENOMEM; - mutex_lock(&config->mutex); + if (trylock) { + if (!mutex_trylock(&config->mutex)) + return -EBUSY; + } else { + mutex_lock(&config->mutex); + } drm_modeset_acquire_init(ctx, 0); + ctx->trylock_only = trylock; retry: ret = drm_modeset_lock(&config->connection_mutex, ctx); @@ -95,13 +95,29 @@ retry: drm_warn_on_modeset_not_all_locked(dev); - return; + return 0; fail: if (ret == -EDEADLK) { drm_modeset_backoff(ctx); 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); @@ -287,7 +303,12 @@ static inline int modeset_lock(struct drm_modeset_lock *lock, 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); } else if (interruptible) { ret = ww_mutex_lock_interruptible(&lock->mutex, &ctx->ww_ctx); diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h index d38e1508f11a..a3f736d24382 100644 --- a/include/drm/drm_modeset_lock.h +++ b/include/drm/drm_modeset_lock.h @@ -53,6 +53,11 @@ struct drm_modeset_acquire_ctx { * list of held locks (drm_modeset_lock) */ struct list_head locked; + + /** + * Trylock mode, use only for panic handlers! + */ + bool trylock_only; }; /** @@ -123,6 +128,7 @@ struct drm_device; struct drm_crtc; 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_lock_crtc(struct drm_crtc *crtc); void drm_modeset_unlock_crtc(struct drm_crtc *crtc);