From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com ([134.134.136.24]:23957 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752459AbcEBJHp (ORCPT ); Mon, 2 May 2016 05:07:45 -0400 Subject: [REBASED PATCH] drm/core: Do not preserve framebuffer on rmfb, v3. 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> Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, Thomas Hellstrom , stable@vger.kernel.org From: Maarten Lankhorst Message-ID: Date: Mon, 2 May 2016 11:07:37 +0200 MIME-Version: 1.0 In-Reply-To: <20160412104208.GC2510@phenom.ffwll.local> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: 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); -- 2.5.5 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maarten Lankhorst Subject: [REBASED PATCH] drm/core: Do not preserve framebuffer on rmfb, v3. Date: Mon, 2 May 2016 11:07:37 +0200 Message-ID: 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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20160412104208.GC2510@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: stable@vger.kernel.org, intel-gfx@lists.freedesktop.org, Thomas Hellstrom , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org SXQgdHVybnMgb3V0IHRoYXQgcHJlc2VydmluZyBmcmFtZWJ1ZmZlcnMgYWZ0ZXIgdGhlIHJtZmIg Y2FsbCBicmVha3MKdm13Z2Z4IHVzZXJzcGFjZS4gVGhpcyB3YXMgb3JpZ2luYWxseSBpbnRyb2R1 Y2VkIGJlY2F1c2UgaXQgd2FzIHRob3VnaHQKbm9ib2R5IHJlbGllZCBvbiB0aGUgYmVoYXZpb3Is IGJ1dCB1bmZvcnR1bmF0ZWx5IGl0IHNlZW1zIHRoZXJlIGFyZQpleGNlcHRpb25zLgoKZHJtX2Zy YW1lYnVmZmVyX3JlbW92ZSBtYXkgZmFpbCB3aXRoIC1FSU5UUiBub3csIHNvIGEgc3RyYWlnaHQg cmV2ZXJ0CmlzIGltcG9zc2libGUuIFRoZXJlIGlzIG5vIHdheSB0byByZW1vdmUgdGhlIGZyYW1l YnVmZmVyIGZyb20gdGhlIGxpc3RzCmFuZCBhY3RpdmUgcGxhbmVzIHdpdGhvdXQgaW50cm9kdWNp bmcgYSByYWNlIGJlY2F1c2Ugb2YgdGhlIGRpZmZlcmVudApsb2NraW5nIHJlcXVpcmVtZW50cy4g SW5zdGVhZCBjYWxsIGRybV9mcmFtZWJ1ZmZlcl9yZW1vdmUgZnJvbSBhCndvcmtxdWV1ZSwgd2hp Y2ggaXMgdW5hZmZlY3RlZCBieSBzaWduYWxzLgoKQ2hhbmdlcyBzaW5jZSB2MToKLSBBZGQgY29t bWVudC4KQ2hhbmdlcyBzaW5jZSB2MjoKLSBBZGQgZmFzdHBhdGggZm9yIHJlZmNvdW50ID0gMS4g KGRhbnZldCkKCkNjOiBzdGFibGVAdmdlci5rZXJuZWwub3JnICN2NC40KwpGaXhlczogMTM4MDMx MzI4MThjICgiZHJtL2NvcmU6IFByZXNlcnZlIHRoZSBmcmFtZWJ1ZmZlciBhZnRlciByZW1vdmlu ZyBpdC4iKQpUZXN0Y2FzZToga21zX2ZsaXAuZmxpcC12cy1ybWZiLWludGVycnVwdGlibGUKUmVm ZXJlbmNlczogaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvYXJjaGl2ZXMvZHJpLWRldmVs LzIwMTYtTWFyY2gvMTAyODc2Lmh0bWwKQ2M6IFRob21hcyBIZWxsc3Ryb20gPHRoZWxsc3Ryb21A dm13YXJlLmNvbT4KQ2M6IERhdmlkIEhlcnJtYW5uIDxkaC5oZXJybWFubkBnbWFpbC5jb20+ClJl dmlld2VkLWJ5OiBEYW5pZWwgVmV0dGVyIDxkYW5pZWwudmV0dGVyQGZmd2xsLmNoPgotLS0KUmVi YXNlZCB2ZXJzaW9uLgoKIGRyaXZlcnMvZ3B1L2RybS9kcm1fY3J0Yy5jIHwgMzIgKysrKysrKysr KysrKysrKysrKysrKysrKysrKysrLS0KIDEgZmlsZSBjaGFuZ2VkLCAzMCBpbnNlcnRpb25zKCsp LCAyIGRlbGV0aW9ucygtKQoKZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fY3J0Yy5j IGIvZHJpdmVycy9ncHUvZHJtL2RybV9jcnRjLmMKaW5kZXggOTYyNmEwY2MwNTBhLi44OGY4NWM5 MGJiZWQgMTAwNjQ0Ci0tLSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fY3J0Yy5jCisrKyBiL2RyaXZl cnMvZ3B1L2RybS9kcm1fY3J0Yy5jCkBAIC0zNDQwLDYgKzM0NDAsMTggQEAgaW50IGRybV9tb2Rl X2FkZGZiMihzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LAogCXJldHVybiAwOwogfQogCitzdHJ1Y3Qg ZHJtX21vZGVfcm1mYl93b3JrIHsKKwlzdHJ1Y3Qgd29ya19zdHJ1Y3Qgd29yazsKKwlzdHJ1Y3Qg ZHJtX2ZyYW1lYnVmZmVyICpmYjsKK307CisKK3N0YXRpYyB2b2lkIGRybV9tb2RlX3JtZmJfd29y a19mbihzdHJ1Y3Qgd29ya19zdHJ1Y3QgKncpCit7CisJc3RydWN0IGRybV9tb2RlX3JtZmJfd29y ayAqYXJnID0gY29udGFpbmVyX29mKHcsIHR5cGVvZigqYXJnKSwgd29yayk7CisKKwlkcm1fZnJh bWVidWZmZXJfcmVtb3ZlKGFyZy0+ZmIpOworfQorCiAvKioKICAqIGRybV9tb2RlX3JtZmIgLSBy ZW1vdmUgYW4gRkIgZnJvbSB0aGUgY29uZmlndXJhdGlvbgogICogQGRldjogZHJtIGRldmljZSBm b3IgdGhlIGlvY3RsCkBAIC0zNDgwLDggKzM0OTIsMjQgQEAgaW50IGRybV9tb2RlX3JtZmIoc3Ry dWN0IGRybV9kZXZpY2UgKmRldiwKIAlsaXN0X2RlbF9pbml0KCZmYi0+ZmlscF9oZWFkKTsKIAlt dXRleF91bmxvY2soJmZpbGVfcHJpdi0+ZmJzX2xvY2spOwogCi0JLyogd2Ugbm93IG93biB0aGUg cmVmZXJlbmNlIHRoYXQgd2FzIHN0b3JlZCBpbiB0aGUgZmJzIGxpc3QgKi8KLQlkcm1fZnJhbWVi dWZmZXJfdW5yZWZlcmVuY2UoZmIpOworCS8qCisJICogd2Ugbm93IG93biB0aGUgcmVmZXJlbmNl IHRoYXQgd2FzIHN0b3JlZCBpbiB0aGUgZmJzIGxpc3QKKwkgKgorCSAqIGRybV9mcmFtZWJ1ZmZl cl9yZW1vdmUgbWF5IGZhaWwgd2l0aCAtRUlOVFIgb24gcGVuZGluZyBzaWduYWxzLAorCSAqIHNv IHJ1biB0aGlzIGluIGEgc2VwYXJhdGUgc3RhY2sgYXMgdGhlcmUncyBubyB3YXkgdG8gY29ycmVj dGx5CisJICogaGFuZGxlIHRoaXMgYWZ0ZXIgdGhlIGZiIGlzIGFscmVhZHkgcmVtb3ZlZCBmcm9t IHRoZSBsb29rdXAgdGFibGUuCisJICovCisJaWYgKGF0b21pY19yZWFkKCZmYi0+cmVmY291bnQu cmVmY291bnQpID4gMSkgeworCQlzdHJ1Y3QgZHJtX21vZGVfcm1mYl93b3JrIGFyZzsKKworCQlJ TklUX1dPUktfT05TVEFDSygmYXJnLndvcmssIGRybV9tb2RlX3JtZmJfd29ya19mbik7CisJCWFy Zy5mYiA9IGZiOworCisJCXNjaGVkdWxlX3dvcmsoJmFyZy53b3JrKTsKKwkJZmx1c2hfd29yaygm YXJnLndvcmspOworCQlkZXN0cm95X3dvcmtfb25fc3RhY2soJmFyZy53b3JrKTsKKwl9IGVsc2UK KwkJZHJtX2ZyYW1lYnVmZmVyX3VucmVmZXJlbmNlKGZiKTsKIAogCS8qIGRyb3AgdGhlIHJlZmVy ZW5jZSB3ZSBwaWNrZWQgdXAgaW4gZnJhbWVidWZmZXIgbG9va3VwICovCiAJZHJtX2ZyYW1lYnVm ZmVyX3VucmVmZXJlbmNlKGZiKTsKLS0gCjIuNS41CgoKX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxA bGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxt YW4vbGlzdGluZm8vZHJpLWRldmVsCg==