Skip to content

Commit 0d6ad54

Browse files
mlankhorstgregkh
authored andcommitted
drm/core: Do not preserve framebuffer on rmfb, v4.
commit f2d580b9a8149735cbc4b59c4a8df60173658140 upstream. 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) Changes since v3: - Rebased. - Restore lastclose framebuffer removal too. Fixes: 1380313 ("drm/core: Preserve the framebuffer after removing it.") Testcase: kms_rmfb_basic 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> Tested-by: Thomas Hellstrom <thellstrom@vmware.com> #v3 Tested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Link: http://patchwork.freedesktop.org/patch/msgid/6c63ca37-0e7e-ac7f-a6d2-c7822e3d611f@linux.intel.com Signed-off-by: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent dbea3ce commit 0d6ad54

1 file changed

Lines changed: 55 additions & 5 deletions

File tree

drivers/gpu/drm/drm_crtc.c

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3316,6 +3316,24 @@ int drm_mode_addfb2(struct drm_device *dev,
33163316
return 0;
33173317
}
33183318

3319+
struct drm_mode_rmfb_work {
3320+
struct work_struct work;
3321+
struct list_head fbs;
3322+
};
3323+
3324+
static void drm_mode_rmfb_work_fn(struct work_struct *w)
3325+
{
3326+
struct drm_mode_rmfb_work *arg = container_of(w, typeof(*arg), work);
3327+
3328+
while (!list_empty(&arg->fbs)) {
3329+
struct drm_framebuffer *fb =
3330+
list_first_entry(&arg->fbs, typeof(*fb), filp_head);
3331+
3332+
list_del_init(&fb->filp_head);
3333+
drm_framebuffer_remove(fb);
3334+
}
3335+
}
3336+
33193337
/**
33203338
* drm_mode_rmfb - remove an FB from the configuration
33213339
* @dev: drm device for the ioctl
@@ -3356,7 +3374,25 @@ int drm_mode_rmfb(struct drm_device *dev,
33563374
mutex_unlock(&dev->mode_config.fb_lock);
33573375
mutex_unlock(&file_priv->fbs_lock);
33583376

3359-
drm_framebuffer_unreference(fb);
3377+
/*
3378+
* we now own the reference that was stored in the fbs list
3379+
*
3380+
* drm_framebuffer_remove may fail with -EINTR on pending signals,
3381+
* so run this in a separate stack as there's no way to correctly
3382+
* handle this after the fb is already removed from the lookup table.
3383+
*/
3384+
if (atomic_read(&fb->refcount.refcount) > 1) {
3385+
struct drm_mode_rmfb_work arg;
3386+
3387+
INIT_WORK_ONSTACK(&arg.work, drm_mode_rmfb_work_fn);
3388+
INIT_LIST_HEAD(&arg.fbs);
3389+
list_add_tail(&fb->filp_head, &arg.fbs);
3390+
3391+
schedule_work(&arg.work);
3392+
flush_work(&arg.work);
3393+
destroy_work_on_stack(&arg.work);
3394+
} else
3395+
drm_framebuffer_unreference(fb);
33603396

33613397
return 0;
33623398

@@ -3509,7 +3545,6 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
35093545
return ret;
35103546
}
35113547

3512-
35133548
/**
35143549
* drm_fb_release - remove and free the FBs on this file
35153550
* @priv: drm file for the ioctl
@@ -3524,6 +3559,9 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
35243559
void drm_fb_release(struct drm_file *priv)
35253560
{
35263561
struct drm_framebuffer *fb, *tfb;
3562+
struct drm_mode_rmfb_work arg;
3563+
3564+
INIT_LIST_HEAD(&arg.fbs);
35273565

35283566
/*
35293567
* When the file gets released that means no one else can access the fb
@@ -3536,10 +3574,22 @@ void drm_fb_release(struct drm_file *priv)
35363574
* at it any more.
35373575
*/
35383576
list_for_each_entry_safe(fb, tfb, &priv->fbs, filp_head) {
3539-
list_del_init(&fb->filp_head);
3577+
if (atomic_read(&fb->refcount.refcount) > 1) {
3578+
list_move_tail(&fb->filp_head, &arg.fbs);
3579+
} else {
3580+
list_del_init(&fb->filp_head);
35403581

3541-
/* This drops the fpriv->fbs reference. */
3542-
drm_framebuffer_unreference(fb);
3582+
/* This drops the fpriv->fbs reference. */
3583+
drm_framebuffer_unreference(fb);
3584+
}
3585+
}
3586+
3587+
if (!list_empty(&arg.fbs)) {
3588+
INIT_WORK_ONSTACK(&arg.work, drm_mode_rmfb_work_fn);
3589+
3590+
schedule_work(&arg.work);
3591+
flush_work(&arg.work);
3592+
destroy_work_on_stack(&arg.work);
35433593
}
35443594
}
35453595

0 commit comments

Comments
 (0)