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@xxxxxxxxxxxxxxx #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@xxxxxxxxxx> > Cc: David Herrmann <dh.herrmann@xxxxxxxxx> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html