From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-lf0-f65.google.com ([209.85.215.65]:35525 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1034477AbcIWNta (ORCPT ); Fri, 23 Sep 2016 09:49:30 -0400 Received: by mail-lf0-f65.google.com with SMTP id s64so5744831lfs.2 for ; Fri, 23 Sep 2016 06:49:30 -0700 (PDT) Date: Fri, 23 Sep 2016 15:49:26 +0200 From: Daniel Vetter To: Chris Wilson Cc: dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, intel-gfx@lists.freedesktop.org, linux-media@vger.kernel.org Subject: Re: [Linaro-mm-sig] [PATCH 10/11] dma-buf: Use seqlock to close RCU race in test_signaled_single Message-ID: <20160923134926.GL3988@dvetter-linux.ger.corp.intel.com> References: <20160829070834.22296-1-chris@chris-wilson.co.uk> <20160829070834.22296-10-chris@chris-wilson.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160829070834.22296-10-chris@chris-wilson.co.uk> Sender: linux-media-owner@vger.kernel.org List-ID: On Mon, Aug 29, 2016 at 08:08:33AM +0100, Chris Wilson wrote: > With the seqlock now extended to cover the lookup of the fence and its > testing, we can perform that testing solely under the seqlock guard and > avoid the effective locking and serialisation of acquiring a reference to > the request. As the fence is RCU protected we know it cannot disappear > as we test it, the same guarantee that made it safe to acquire the > reference previously. The seqlock tests whether the fence was replaced > as we are testing it telling us whether or not we can trust the result > (if not, we just repeat the test until stable). > > Signed-off-by: Chris Wilson > Cc: Sumit Semwal > Cc: linux-media@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: linaro-mm-sig@lists.linaro.org Not entirely sure this is safe for non-i915 drivers. We might now call ->signalled on a zombie fence (i.e. refcount == 0, but not yet kfreed). i915 can do that, but other drivers might go boom. I think in generic code we can't do these kind of tricks unfortunately. -Daniel > --- > drivers/dma-buf/reservation.c | 32 ++++---------------------------- > 1 file changed, 4 insertions(+), 28 deletions(-) > > diff --git a/drivers/dma-buf/reservation.c b/drivers/dma-buf/reservation.c > index e74493e7332b..1ddffa5adb5a 100644 > --- a/drivers/dma-buf/reservation.c > +++ b/drivers/dma-buf/reservation.c > @@ -442,24 +442,6 @@ unlock_retry: > } > EXPORT_SYMBOL_GPL(reservation_object_wait_timeout_rcu); > > - > -static inline int > -reservation_object_test_signaled_single(struct fence *passed_fence) > -{ > - struct fence *fence, *lfence = passed_fence; > - int ret = 1; > - > - if (!test_bit(FENCE_FLAG_SIGNALED_BIT, &lfence->flags)) { > - fence = fence_get_rcu(lfence); > - if (!fence) > - return -1; > - > - ret = !!fence_is_signaled(fence); > - fence_put(fence); > - } > - return ret; > -} > - > /** > * reservation_object_test_signaled_rcu - Test if a reservation object's > * fences have been signaled. > @@ -474,7 +456,7 @@ bool reservation_object_test_signaled_rcu(struct reservation_object *obj, > bool test_all) > { > unsigned seq, shared_count; > - int ret; > + bool ret; > > rcu_read_lock(); > retry: > @@ -494,10 +476,8 @@ retry: > for (i = 0; i < shared_count; ++i) { > struct fence *fence = rcu_dereference(fobj->shared[i]); > > - ret = reservation_object_test_signaled_single(fence); > - if (ret < 0) > - goto retry; > - else if (!ret) > + ret = fence_is_signaled(fence); > + if (!ret) > break; > } > > @@ -509,11 +489,7 @@ retry: > struct fence *fence_excl = rcu_dereference(obj->fence_excl); > > if (fence_excl) { > - ret = reservation_object_test_signaled_single( > - fence_excl); > - if (ret < 0) > - goto retry; > - > + ret = fence_is_signaled(fence_excl); > if (read_seqcount_retry(&obj->seq, seq)) > goto retry; > } > -- > 2.9.3 > > _______________________________________________ > Linaro-mm-sig mailing list > Linaro-mm-sig@lists.linaro.org > https://lists.linaro.org/mailman/listinfo/linaro-mm-sig -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [Linaro-mm-sig] [PATCH 10/11] dma-buf: Use seqlock to close RCU race in test_signaled_single Date: Fri, 23 Sep 2016 15:49:26 +0200 Message-ID: <20160923134926.GL3988@dvetter-linux.ger.corp.intel.com> References: <20160829070834.22296-1-chris@chris-wilson.co.uk> <20160829070834.22296-10-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-lf0-x241.google.com (mail-lf0-x241.google.com [IPv6:2a00:1450:4010:c07::241]) by gabe.freedesktop.org (Postfix) with ESMTPS id AC7676EA8E for ; Fri, 23 Sep 2016 13:49:30 +0000 (UTC) Received: by mail-lf0-x241.google.com with SMTP id l131so5740635lfl.0 for ; Fri, 23 Sep 2016 06:49:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20160829070834.22296-10-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Chris Wilson Cc: linaro-mm-sig@lists.linaro.org, intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org T24gTW9uLCBBdWcgMjksIDIwMTYgYXQgMDg6MDg6MzNBTSArMDEwMCwgQ2hyaXMgV2lsc29uIHdy b3RlOgo+IFdpdGggdGhlIHNlcWxvY2sgbm93IGV4dGVuZGVkIHRvIGNvdmVyIHRoZSBsb29rdXAg b2YgdGhlIGZlbmNlIGFuZCBpdHMKPiB0ZXN0aW5nLCB3ZSBjYW4gcGVyZm9ybSB0aGF0IHRlc3Rp bmcgc29sZWx5IHVuZGVyIHRoZSBzZXFsb2NrIGd1YXJkIGFuZAo+IGF2b2lkIHRoZSBlZmZlY3Rp dmUgbG9ja2luZyBhbmQgc2VyaWFsaXNhdGlvbiBvZiBhY3F1aXJpbmcgYSByZWZlcmVuY2UgdG8K PiB0aGUgcmVxdWVzdC4gIEFzIHRoZSBmZW5jZSBpcyBSQ1UgcHJvdGVjdGVkIHdlIGtub3cgaXQg Y2Fubm90IGRpc2FwcGVhcgo+IGFzIHdlIHRlc3QgaXQsIHRoZSBzYW1lIGd1YXJhbnRlZSB0aGF0 IG1hZGUgaXQgc2FmZSB0byBhY3F1aXJlIHRoZQo+IHJlZmVyZW5jZSBwcmV2aW91c2x5LiAgVGhl IHNlcWxvY2sgdGVzdHMgd2hldGhlciB0aGUgZmVuY2Ugd2FzIHJlcGxhY2VkCj4gYXMgd2UgYXJl IHRlc3RpbmcgaXQgdGVsbGluZyB1cyB3aGV0aGVyIG9yIG5vdCB3ZSBjYW4gdHJ1c3QgdGhlIHJl c3VsdAo+IChpZiBub3QsIHdlIGp1c3QgcmVwZWF0IHRoZSB0ZXN0IHVudGlsIHN0YWJsZSkuCj4g Cj4gU2lnbmVkLW9mZi1ieTogQ2hyaXMgV2lsc29uIDxjaHJpc0BjaHJpcy13aWxzb24uY28udWs+ Cj4gQ2M6IFN1bWl0IFNlbXdhbCA8c3VtaXQuc2Vtd2FsQGxpbmFyby5vcmc+Cj4gQ2M6IGxpbnV4 LW1lZGlhQHZnZXIua2VybmVsLm9yZwo+IENjOiBkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Au b3JnCj4gQ2M6IGxpbmFyby1tbS1zaWdAbGlzdHMubGluYXJvLm9yZwoKTm90IGVudGlyZWx5IHN1 cmUgdGhpcyBpcyBzYWZlIGZvciBub24taTkxNSBkcml2ZXJzLiBXZSBtaWdodCBub3cgY2FsbAot PnNpZ25hbGxlZCBvbiBhIHpvbWJpZSBmZW5jZSAoaS5lLiByZWZjb3VudCA9PSAwLCBidXQgbm90 IHlldCBrZnJlZWQpLgppOTE1IGNhbiBkbyB0aGF0LCBidXQgb3RoZXIgZHJpdmVycyBtaWdodCBn byBib29tLgoKSSB0aGluayBpbiBnZW5lcmljIGNvZGUgd2UgY2FuJ3QgZG8gdGhlc2Uga2luZCBv ZiB0cmlja3MgdW5mb3J0dW5hdGVseS4KLURhbmllbAoKPiAtLS0KPiAgZHJpdmVycy9kbWEtYnVm L3Jlc2VydmF0aW9uLmMgfCAzMiArKysrLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLQo+ICAx IGZpbGUgY2hhbmdlZCwgNCBpbnNlcnRpb25zKCspLCAyOCBkZWxldGlvbnMoLSkKPiAKPiBkaWZm IC0tZ2l0IGEvZHJpdmVycy9kbWEtYnVmL3Jlc2VydmF0aW9uLmMgYi9kcml2ZXJzL2RtYS1idWYv cmVzZXJ2YXRpb24uYwo+IGluZGV4IGU3NDQ5M2U3MzMyYi4uMWRkZmZhNWFkYjVhIDEwMDY0NAo+ IC0tLSBhL2RyaXZlcnMvZG1hLWJ1Zi9yZXNlcnZhdGlvbi5jCj4gKysrIGIvZHJpdmVycy9kbWEt YnVmL3Jlc2VydmF0aW9uLmMKPiBAQCAtNDQyLDI0ICs0NDIsNiBAQCB1bmxvY2tfcmV0cnk6Cj4g IH0KPiAgRVhQT1JUX1NZTUJPTF9HUEwocmVzZXJ2YXRpb25fb2JqZWN0X3dhaXRfdGltZW91dF9y Y3UpOwo+ICAKPiAtCj4gLXN0YXRpYyBpbmxpbmUgaW50Cj4gLXJlc2VydmF0aW9uX29iamVjdF90 ZXN0X3NpZ25hbGVkX3NpbmdsZShzdHJ1Y3QgZmVuY2UgKnBhc3NlZF9mZW5jZSkKPiAtewo+IC0J c3RydWN0IGZlbmNlICpmZW5jZSwgKmxmZW5jZSA9IHBhc3NlZF9mZW5jZTsKPiAtCWludCByZXQg PSAxOwo+IC0KPiAtCWlmICghdGVzdF9iaXQoRkVOQ0VfRkxBR19TSUdOQUxFRF9CSVQsICZsZmVu Y2UtPmZsYWdzKSkgewo+IC0JCWZlbmNlID0gZmVuY2VfZ2V0X3JjdShsZmVuY2UpOwo+IC0JCWlm ICghZmVuY2UpCj4gLQkJCXJldHVybiAtMTsKPiAtCj4gLQkJcmV0ID0gISFmZW5jZV9pc19zaWdu YWxlZChmZW5jZSk7Cj4gLQkJZmVuY2VfcHV0KGZlbmNlKTsKPiAtCX0KPiAtCXJldHVybiByZXQ7 Cj4gLX0KPiAtCj4gIC8qKgo+ICAgKiByZXNlcnZhdGlvbl9vYmplY3RfdGVzdF9zaWduYWxlZF9y Y3UgLSBUZXN0IGlmIGEgcmVzZXJ2YXRpb24gb2JqZWN0J3MKPiAgICogZmVuY2VzIGhhdmUgYmVl biBzaWduYWxlZC4KPiBAQCAtNDc0LDcgKzQ1Niw3IEBAIGJvb2wgcmVzZXJ2YXRpb25fb2JqZWN0 X3Rlc3Rfc2lnbmFsZWRfcmN1KHN0cnVjdCByZXNlcnZhdGlvbl9vYmplY3QgKm9iaiwKPiAgCQkJ CQkgIGJvb2wgdGVzdF9hbGwpCj4gIHsKPiAgCXVuc2lnbmVkIHNlcSwgc2hhcmVkX2NvdW50Owo+ IC0JaW50IHJldDsKPiArCWJvb2wgcmV0Owo+ICAKPiAgCXJjdV9yZWFkX2xvY2soKTsKPiAgcmV0 cnk6Cj4gQEAgLTQ5NCwxMCArNDc2LDggQEAgcmV0cnk6Cj4gIAkJZm9yIChpID0gMDsgaSA8IHNo YXJlZF9jb3VudDsgKytpKSB7Cj4gIAkJCXN0cnVjdCBmZW5jZSAqZmVuY2UgPSByY3VfZGVyZWZl cmVuY2UoZm9iai0+c2hhcmVkW2ldKTsKPiAgCj4gLQkJCXJldCA9IHJlc2VydmF0aW9uX29iamVj dF90ZXN0X3NpZ25hbGVkX3NpbmdsZShmZW5jZSk7Cj4gLQkJCWlmIChyZXQgPCAwKQo+IC0JCQkJ Z290byByZXRyeTsKPiAtCQkJZWxzZSBpZiAoIXJldCkKPiArCQkJcmV0ID0gZmVuY2VfaXNfc2ln bmFsZWQoZmVuY2UpOwo+ICsJCQlpZiAoIXJldCkKPiAgCQkJCWJyZWFrOwo+ICAJCX0KPiAgCj4g QEAgLTUwOSwxMSArNDg5LDcgQEAgcmV0cnk6Cj4gIAkJc3RydWN0IGZlbmNlICpmZW5jZV9leGNs ID0gcmN1X2RlcmVmZXJlbmNlKG9iai0+ZmVuY2VfZXhjbCk7Cj4gIAo+ICAJCWlmIChmZW5jZV9l eGNsKSB7Cj4gLQkJCXJldCA9IHJlc2VydmF0aW9uX29iamVjdF90ZXN0X3NpZ25hbGVkX3Npbmds ZSgKPiAtCQkJCQkJCQlmZW5jZV9leGNsKTsKPiAtCQkJaWYgKHJldCA8IDApCj4gLQkJCQlnb3Rv IHJldHJ5Owo+IC0KPiArCQkJcmV0ID0gZmVuY2VfaXNfc2lnbmFsZWQoZmVuY2VfZXhjbCk7Cj4g IAkJCWlmIChyZWFkX3NlcWNvdW50X3JldHJ5KCZvYmotPnNlcSwgc2VxKSkKPiAgCQkJCWdvdG8g cmV0cnk7Cj4gIAkJfQo+IC0tIAo+IDIuOS4zCj4gCj4gX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX18KPiBMaW5hcm8tbW0tc2lnIG1haWxpbmcgbGlzdAo+IExp bmFyby1tbS1zaWdAbGlzdHMubGluYXJvLm9yZwo+IGh0dHBzOi8vbGlzdHMubGluYXJvLm9yZy9t YWlsbWFuL2xpc3RpbmZvL2xpbmFyby1tbS1zaWcKCi0tIApEYW5pZWwgVmV0dGVyClNvZnR3YXJl IEVuZ2luZWVyLCBJbnRlbCBDb3Jwb3JhdGlvbgpodHRwOi8vYmxvZy5mZndsbC5jaApfX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGlu ZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVl ZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK