From patchwork Mon May 2 09:07:37 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Maarten Lankhorst X-Patchwork-Id: 8990711 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 0E3D99F1D3 for ; Mon, 2 May 2016 09:07:48 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1E26920211 for ; Mon, 2 May 2016 09:07:47 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 2A1B120145 for ; Mon, 2 May 2016 09:07:46 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D452F6E56B; Mon, 2 May 2016 09:07:44 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id F0C006E56B; Mon, 2 May 2016 09:07:43 +0000 (UTC) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 02 May 2016 02:07:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,566,1455004800"; d="scan'208";a="956992259" Received: from idobaron-mobl2.ger.corp.intel.com (HELO patser.lan) ([10.252.22.123]) by fmsmga001.fm.intel.com with ESMTP; 02 May 2016 02:07:44 -0700 To: Daniel Vetter References: <1458640720-2526-1-git-send-email-maarten.lankhorst@linux.intel.com> <1459423563-27558-1-git-send-email-maarten.lankhorst@linux.intel.com> <20160412104208.GC2510@phenom.ffwll.local> From: Maarten Lankhorst Message-ID: Date: Mon, 2 May 2016 11:07:37 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <20160412104208.GC2510@phenom.ffwll.local> Cc: stable@vger.kernel.org, intel-gfx@lists.freedesktop.org, Thomas Hellstrom , dri-devel@lists.freedesktop.org Subject: [Intel-gfx] [REBASED PATCH] drm/core: Do not preserve framebuffer on rmfb, v3. X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-5.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, 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 It turns out that preserving framebuffers after the rmfb call breaks vmwgfx userspace. This was originally introduced because it was thought nobody relied on the behavior, but unfortunately it seems there are exceptions. drm_framebuffer_remove may fail with -EINTR now, so a straight revert is impossible. There is no way to remove the framebuffer from the lists and active planes without introducing a race because of the different locking requirements. Instead call drm_framebuffer_remove from a workqueue, which is unaffected by signals. Changes since v1: - Add comment. Changes since v2: - Add fastpath for refcount = 1. (danvet) Cc: stable@vger.kernel.org #v4.4+ Fixes: 13803132818c ("drm/core: Preserve the framebuffer after removing it.") Testcase: kms_flip.flip-vs-rmfb-interruptible References: https://lists.freedesktop.org/archives/dri-devel/2016-March/102876.html Cc: Thomas Hellstrom Cc: David Herrmann Reviewed-by: Daniel Vetter --- Rebased version. drivers/gpu/drm/drm_crtc.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 9626a0cc050a..88f85c90bbed 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -3440,6 +3440,18 @@ int drm_mode_addfb2(struct drm_device *dev, return 0; } +struct drm_mode_rmfb_work { + struct work_struct work; + struct drm_framebuffer *fb; +}; + +static void drm_mode_rmfb_work_fn(struct work_struct *w) +{ + struct drm_mode_rmfb_work *arg = container_of(w, typeof(*arg), work); + + drm_framebuffer_remove(arg->fb); +} + /** * drm_mode_rmfb - remove an FB from the configuration * @dev: drm device for the ioctl @@ -3480,8 +3492,24 @@ int drm_mode_rmfb(struct drm_device *dev, list_del_init(&fb->filp_head); mutex_unlock(&file_priv->fbs_lock); - /* we now own the reference that was stored in the fbs list */ - drm_framebuffer_unreference(fb); + /* + * we now own the reference that was stored in the fbs list + * + * drm_framebuffer_remove may fail with -EINTR on pending signals, + * so run this in a separate stack as there's no way to correctly + * handle this after the fb is already removed from the lookup table. + */ + if (atomic_read(&fb->refcount.refcount) > 1) { + struct drm_mode_rmfb_work arg; + + INIT_WORK_ONSTACK(&arg.work, drm_mode_rmfb_work_fn); + arg.fb = fb; + + schedule_work(&arg.work); + flush_work(&arg.work); + destroy_work_on_stack(&arg.work); + } else + drm_framebuffer_unreference(fb); /* drop the reference we picked up in framebuffer lookup */ drm_framebuffer_unreference(fb);