From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, HK_RANDOM_FROM,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 95781C7618F for ; Tue, 16 Jul 2019 15:25:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 7510E21743 for ; Tue, 16 Jul 2019 15:25:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387880AbfGPPZY (ORCPT ); Tue, 16 Jul 2019 11:25:24 -0400 Received: from mga05.intel.com ([192.55.52.43]:1675 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728421AbfGPPZY (ORCPT ); Tue, 16 Jul 2019 11:25:24 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Jul 2019 08:25:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,498,1557212400"; d="scan'208";a="158167369" Received: from esulliva-mobl.ger.corp.intel.com (HELO [10.251.94.109]) ([10.251.94.109]) by orsmga007.jf.intel.com with ESMTP; 16 Jul 2019 08:25:22 -0700 Subject: Re: [Intel-gfx] [PATCH 1/5] drm/i915/userptr: Beware recursive lock_page() To: Chris Wilson , intel-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org References: <20190716124931.5870-1-chris@chris-wilson.co.uk> From: Tvrtko Ursulin Organization: Intel Corporation UK Plc Message-ID: Date: Tue, 16 Jul 2019 16:25:22 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <20190716124931.5870-1-chris@chris-wilson.co.uk> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org On 16/07/2019 13:49, Chris Wilson wrote: > Following a try_to_unmap() we may want to remove the userptr and so call > put_pages(). However, try_to_unmap() acquires the page lock and so we > must avoid recursively locking the pages ourselves -- which means that > we cannot safely acquire the lock around set_page_dirty(). Since we > can't be sure of the lock, we have to risk skip dirtying the page, or > else risk calling set_page_dirty() without a lock and so risk fs > corruption. So if trylock randomly fail we get data corruption in whatever data set application is working on, which is what the original patch was trying to avoid? Are we able to detect the backing store type so at least we don't risk skipping set_page_dirty with anonymous/shmemfs? Regards, Tvrtko > Reported-by: Lionel Landwerlin > Fixes: cb6d7c7dc7ff ("drm/i915/userptr: Acquire the page lock around set_page_dirty()") > Signed-off-by: Chris Wilson > Cc: Lionel Landwerlin > Cc: Tvrtko Ursulin > Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > index b9d2bb15e4a6..1ad2047a6dbd 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c > @@ -672,7 +672,7 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj, > obj->mm.dirty = false; > > for_each_sgt_page(page, sgt_iter, pages) { > - if (obj->mm.dirty) > + if (obj->mm.dirty && trylock_page(page)) { > /* > * As this may not be anonymous memory (e.g. shmem) > * but exist on a real mapping, we have to lock > @@ -680,8 +680,20 @@ i915_gem_userptr_put_pages(struct drm_i915_gem_object *obj, > * the page reference is not sufficient to > * prevent the inode from being truncated. > * Play safe and take the lock. > + * > + * However...! > + * > + * The mmu-notifier can be invalidated for a > + * migrate_page, that is alreadying holding the lock > + * on the page. Such a try_to_unmap() will result > + * in us calling put_pages() and so recursively try > + * to lock the page. We avoid that deadlock with > + * a trylock_page() and in exchange we risk missing > + * some page dirtying. > */ > - set_page_dirty_lock(page); > + set_page_dirty(page); > + unlock_page(page); > + } > > mark_page_accessed(page); > put_page(page); > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tvrtko Ursulin Subject: Re: [PATCH 1/5] drm/i915/userptr: Beware recursive lock_page() Date: Tue, 16 Jul 2019 16:25:22 +0100 Message-ID: References: <20190716124931.5870-1-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id B02D86E140 for ; Tue, 16 Jul 2019 15:25:24 +0000 (UTC) In-Reply-To: <20190716124931.5870-1-chris@chris-wilson.co.uk> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , intel-gfx@lists.freedesktop.org Cc: stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org Ck9uIDE2LzA3LzIwMTkgMTM6NDksIENocmlzIFdpbHNvbiB3cm90ZToKPiBGb2xsb3dpbmcgYSB0 cnlfdG9fdW5tYXAoKSB3ZSBtYXkgd2FudCB0byByZW1vdmUgdGhlIHVzZXJwdHIgYW5kIHNvIGNh bGwKPiBwdXRfcGFnZXMoKS4gSG93ZXZlciwgdHJ5X3RvX3VubWFwKCkgYWNxdWlyZXMgdGhlIHBh Z2UgbG9jayBhbmQgc28gd2UKPiBtdXN0IGF2b2lkIHJlY3Vyc2l2ZWx5IGxvY2tpbmcgdGhlIHBh Z2VzIG91cnNlbHZlcyAtLSB3aGljaCBtZWFucyB0aGF0Cj4gd2UgY2Fubm90IHNhZmVseSBhY3F1 aXJlIHRoZSBsb2NrIGFyb3VuZCBzZXRfcGFnZV9kaXJ0eSgpLiBTaW5jZSB3ZQo+IGNhbid0IGJl IHN1cmUgb2YgdGhlIGxvY2ssIHdlIGhhdmUgdG8gcmlzayBza2lwIGRpcnR5aW5nIHRoZSBwYWdl LCBvcgo+IGVsc2UgcmlzayBjYWxsaW5nIHNldF9wYWdlX2RpcnR5KCkgd2l0aG91dCBhIGxvY2sg YW5kIHNvIHJpc2sgZnMKPiBjb3JydXB0aW9uLgoKU28gaWYgdHJ5bG9jayByYW5kb21seSBmYWls IHdlIGdldCBkYXRhIGNvcnJ1cHRpb24gaW4gd2hhdGV2ZXIgZGF0YSBzZXQgCmFwcGxpY2F0aW9u IGlzIHdvcmtpbmcgb24sIHdoaWNoIGlzIHdoYXQgdGhlIG9yaWdpbmFsIHBhdGNoIHdhcyB0cnlp bmcgCnRvIGF2b2lkPyBBcmUgd2UgYWJsZSB0byBkZXRlY3QgdGhlIGJhY2tpbmcgc3RvcmUgdHlw ZSBzbyBhdCBsZWFzdCB3ZSAKZG9uJ3QgcmlzayBza2lwcGluZyBzZXRfcGFnZV9kaXJ0eSB3aXRo IGFub255bW91cy9zaG1lbWZzPwoKUmVnYXJkcywKClR2cnRrbwoKPiBSZXBvcnRlZC1ieTogTGlv bmVsIExhbmR3ZXJsaW4gPGxpb25lbC5nLmxhbmR3ZXJsaW5AaW50ZWwuY29tPgo+IEZpeGVzOiBj YjZkN2M3ZGM3ZmYgKCJkcm0vaTkxNS91c2VycHRyOiBBY3F1aXJlIHRoZSBwYWdlIGxvY2sgYXJv dW5kIHNldF9wYWdlX2RpcnR5KCkiKQo+IFNpZ25lZC1vZmYtYnk6IENocmlzIFdpbHNvbiA8Y2hy aXNAY2hyaXMtd2lsc29uLmNvLnVrPgo+IENjOiBMaW9uZWwgTGFuZHdlcmxpbiA8bGlvbmVsLmcu bGFuZHdlcmxpbkBpbnRlbC5jb20+Cj4gQ2M6IFR2cnRrbyBVcnN1bGluIDx0dnJ0a28udXJzdWxp bkBpbnRlbC5jb20+Cj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcKPiAtLS0KPiAgIGRyaXZl cnMvZ3B1L2RybS9pOTE1L2dlbS9pOTE1X2dlbV91c2VycHRyLmMgfCAxNiArKysrKysrKysrKysr Ky0tCj4gICAxIGZpbGUgY2hhbmdlZCwgMTQgaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkK PiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvZ2VtL2k5MTVfZ2VtX3VzZXJw dHIuYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2dlbS9pOTE1X2dlbV91c2VycHRyLmMKPiBpbmRl eCBiOWQyYmIxNWU0YTYuLjFhZDIwNDdhNmRiZCAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9k cm0vaTkxNS9nZW0vaTkxNV9nZW1fdXNlcnB0ci5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5 MTUvZ2VtL2k5MTVfZ2VtX3VzZXJwdHIuYwo+IEBAIC02NzIsNyArNjcyLDcgQEAgaTkxNV9nZW1f dXNlcnB0cl9wdXRfcGFnZXMoc3RydWN0IGRybV9pOTE1X2dlbV9vYmplY3QgKm9iaiwKPiAgIAkJ b2JqLT5tbS5kaXJ0eSA9IGZhbHNlOwo+ICAgCj4gICAJZm9yX2VhY2hfc2d0X3BhZ2UocGFnZSwg c2d0X2l0ZXIsIHBhZ2VzKSB7Cj4gLQkJaWYgKG9iai0+bW0uZGlydHkpCj4gKwkJaWYgKG9iai0+ bW0uZGlydHkgJiYgdHJ5bG9ja19wYWdlKHBhZ2UpKSB7Cj4gICAJCQkvKgo+ICAgCQkJICogQXMg dGhpcyBtYXkgbm90IGJlIGFub255bW91cyBtZW1vcnkgKGUuZy4gc2htZW0pCj4gICAJCQkgKiBi dXQgZXhpc3Qgb24gYSByZWFsIG1hcHBpbmcsIHdlIGhhdmUgdG8gbG9jawo+IEBAIC02ODAsOCAr NjgwLDIwIEBAIGk5MTVfZ2VtX3VzZXJwdHJfcHV0X3BhZ2VzKHN0cnVjdCBkcm1faTkxNV9nZW1f b2JqZWN0ICpvYmosCj4gICAJCQkgKiB0aGUgcGFnZSByZWZlcmVuY2UgaXMgbm90IHN1ZmZpY2ll bnQgdG8KPiAgIAkJCSAqIHByZXZlbnQgdGhlIGlub2RlIGZyb20gYmVpbmcgdHJ1bmNhdGVkLgo+ ICAgCQkJICogUGxheSBzYWZlIGFuZCB0YWtlIHRoZSBsb2NrLgo+ICsJCQkgKgo+ICsJCQkgKiBI b3dldmVyLi4uIQo+ICsJCQkgKgo+ICsJCQkgKiBUaGUgbW11LW5vdGlmaWVyIGNhbiBiZSBpbnZh bGlkYXRlZCBmb3IgYQo+ICsJCQkgKiBtaWdyYXRlX3BhZ2UsIHRoYXQgaXMgYWxyZWFkeWluZyBo b2xkaW5nIHRoZSBsb2NrCj4gKwkJCSAqIG9uIHRoZSBwYWdlLiBTdWNoIGEgdHJ5X3RvX3VubWFw KCkgd2lsbCByZXN1bHQKPiArCQkJICogaW4gdXMgY2FsbGluZyBwdXRfcGFnZXMoKSBhbmQgc28g cmVjdXJzaXZlbHkgdHJ5Cj4gKwkJCSAqIHRvIGxvY2sgdGhlIHBhZ2UuIFdlIGF2b2lkIHRoYXQg ZGVhZGxvY2sgd2l0aAo+ICsJCQkgKiBhIHRyeWxvY2tfcGFnZSgpIGFuZCBpbiBleGNoYW5nZSB3 ZSByaXNrIG1pc3NpbmcKPiArCQkJICogc29tZSBwYWdlIGRpcnR5aW5nLgo+ICAgCQkJICovCj4g LQkJCXNldF9wYWdlX2RpcnR5X2xvY2socGFnZSk7Cj4gKwkJCXNldF9wYWdlX2RpcnR5KHBhZ2Up Owo+ICsJCQl1bmxvY2tfcGFnZShwYWdlKTsKPiArCQl9Cj4gICAKPiAgIAkJbWFya19wYWdlX2Fj Y2Vzc2VkKHBhZ2UpOwo+ICAgCQlwdXRfcGFnZShwYWdlKTsKPiAKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRl bC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3Jn L21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4