From: Daniel Vetter <daniel@ffwll.ch> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Thomas Hellstrom <thellstrom@vmware.com>, stable@vger.kernel.org Subject: Re: [PATCH] drm/core: Do not preserve framebuffer on rmfb, v3. Date: Tue, 12 Apr 2016 12:42:41 +0200 [thread overview] Message-ID: <20160412104208.GC2510@phenom.ffwll.local> (raw) In-Reply-To: <1459423563-27558-1-git-send-email-maarten.lankhorst@linux.intel.com> 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 <thellstrom@vmware.com> > Cc: David Herrmann <dh.herrmann@gmail.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> 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
WARNING: multiple messages have this Message-ID (diff)
From: Daniel Vetter <daniel@ffwll.ch> To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: stable@vger.kernel.org, intel-gfx@lists.freedesktop.org, Thomas Hellstrom <thellstrom@vmware.com>, dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm/core: Do not preserve framebuffer on rmfb, v3. Date: Tue, 12 Apr 2016 12:42:41 +0200 [thread overview] Message-ID: <20160412104208.GC2510@phenom.ffwll.local> (raw) In-Reply-To: <1459423563-27558-1-git-send-email-maarten.lankhorst@linux.intel.com> 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 <thellstrom@vmware.com> > Cc: David Herrmann <dh.herrmann@gmail.com> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2016-04-12 10:42 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-03-22 9:58 [PATCH] drm/core: Do not preserve framebuffer on rmfb, v2 Maarten Lankhorst 2016-03-22 11:35 ` ✗ Fi.CI.BAT: failure for " Patchwork 2016-03-31 11:26 ` [PATCH] drm/core: Do not preserve framebuffer on rmfb, v3 Maarten Lankhorst 2016-04-11 7:30 ` Maarten Lankhorst 2016-04-12 10:42 ` Daniel Vetter [this message] 2016-04-12 10:42 ` Daniel Vetter 2016-05-02 9:07 ` [REBASED PATCH] " Maarten Lankhorst 2016-05-02 9:07 ` Maarten Lankhorst 2016-05-03 12:01 ` [PATCH] " Thomas Hellstrom 2016-05-04 12:10 ` [PATCH i-g-t] tests/kms: Add test for testing rmfb framebuffer removal handling Maarten Lankhorst 2016-05-05 9:11 ` [Intel-gfx] " Tvrtko Ursulin 2016-05-05 11:50 ` Daniel Vetter 2016-05-04 12:38 ` [PATCH] drm/core: Do not preserve framebuffer on rmfb, v4 Maarten Lankhorst 2016-05-05 9:07 ` Tvrtko Ursulin 2016-05-05 11:51 ` [Intel-gfx] " Daniel Vetter
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160412104208.GC2510@phenom.ffwll.local \ --to=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=intel-gfx@lists.freedesktop.org \ --cc=maarten.lankhorst@linux.intel.com \ --cc=stable@vger.kernel.org \ --cc=thellstrom@vmware.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.