From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f42.google.com ([74.125.82.42]:33451 "EHLO mail-wm0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756582AbcDLKmZ (ORCPT ); Tue, 12 Apr 2016 06:42:25 -0400 Received: by mail-wm0-f42.google.com with SMTP id f198so181772249wme.0 for ; Tue, 12 Apr 2016 03:42:24 -0700 (PDT) Date: Tue, 12 Apr 2016 12:42:41 +0200 From: Daniel Vetter To: Maarten Lankhorst Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Thomas Hellstrom , stable@vger.kernel.org Subject: Re: [PATCH] drm/core: Do not preserve framebuffer on rmfb, v3. Message-ID: <20160412104208.GC2510@phenom.ffwll.local> References: <1458640720-2526-1-git-send-email-maarten.lankhorst@linux.intel.com> <1459423563-27558-1-git-send-email-maarten.lankhorst@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1459423563-27558-1-git-send-email-maarten.lankhorst@linux.intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Mar 31, 2016 at 01:26:03PM +0200, Maarten Lankhorst wrote: > 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 But definitely want a t-b from Thomas before applying, since he reported this regression. -Daniel > --- > drivers/gpu/drm/drm_crtc.c | 29 ++++++++++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 55ffde5a3a4a..743bece1f579 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -3434,6 +3434,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 > @@ -3474,7 +3486,22 @@ int drm_mode_rmfb(struct drm_device *dev, > mutex_unlock(&dev->mode_config.fb_lock); > mutex_unlock(&file_priv->fbs_lock); > > - drm_framebuffer_unreference(fb); > + /* > + * 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); > > return 0; > > -- > 2.1.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch