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: Sun, 25 Sep 2016 22:43:08 +0200 Message-ID: <20160925204308.GP20761@phenom.ffwll.local> References: <20160829070834.22296-1-chris@chris-wilson.co.uk> <20160829070834.22296-10-chris@chris-wilson.co.uk> <20160923134926.GL3988@dvetter-linux.ger.corp.intel.com> <20160923140232.GD28107@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x244.google.com (mail-wm0-x244.google.com [IPv6:2a00:1450:400c:c09::244]) by gabe.freedesktop.org (Postfix) with ESMTPS id 28B596E269 for ; Sun, 25 Sep 2016 20:43:13 +0000 (UTC) Received: by mail-wm0-x244.google.com with SMTP id 133so11221934wmq.2 for ; Sun, 25 Sep 2016 13:43:13 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20160923140232.GD28107@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Chris Wilson , Daniel Vetter , dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, intel-gfx@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBTZXAgMjMsIDIwMTYgYXQgMDM6MDI6MzJQTSArMDEwMCwgQ2hyaXMgV2lsc29uIHdy b3RlOgo+IE9uIEZyaSwgU2VwIDIzLCAyMDE2IGF0IDAzOjQ5OjI2UE0gKzAyMDAsIERhbmllbCBW ZXR0ZXIgd3JvdGU6Cj4gPiBPbiBNb24sIEF1ZyAyOSwgMjAxNiBhdCAwODowODozM0FNICswMTAw LCBDaHJpcyBXaWxzb24gd3JvdGU6Cj4gPiA+IFdpdGggdGhlIHNlcWxvY2sgbm93IGV4dGVuZGVk IHRvIGNvdmVyIHRoZSBsb29rdXAgb2YgdGhlIGZlbmNlIGFuZCBpdHMKPiA+ID4gdGVzdGluZywg d2UgY2FuIHBlcmZvcm0gdGhhdCB0ZXN0aW5nIHNvbGVseSB1bmRlciB0aGUgc2VxbG9jayBndWFy ZCBhbmQKPiA+ID4gYXZvaWQgdGhlIGVmZmVjdGl2ZSBsb2NraW5nIGFuZCBzZXJpYWxpc2F0aW9u IG9mIGFjcXVpcmluZyBhIHJlZmVyZW5jZSB0bwo+ID4gPiB0aGUgcmVxdWVzdC4gIEFzIHRoZSBm ZW5jZSBpcyBSQ1UgcHJvdGVjdGVkIHdlIGtub3cgaXQgY2Fubm90IGRpc2FwcGVhcgo+ID4gPiBh cyB3ZSB0ZXN0IGl0LCB0aGUgc2FtZSBndWFyYW50ZWUgdGhhdCBtYWRlIGl0IHNhZmUgdG8gYWNx dWlyZSB0aGUKPiA+ID4gcmVmZXJlbmNlIHByZXZpb3VzbHkuICBUaGUgc2VxbG9jayB0ZXN0cyB3 aGV0aGVyIHRoZSBmZW5jZSB3YXMgcmVwbGFjZWQKPiA+ID4gYXMgd2UgYXJlIHRlc3RpbmcgaXQg dGVsbGluZyB1cyB3aGV0aGVyIG9yIG5vdCB3ZSBjYW4gdHJ1c3QgdGhlIHJlc3VsdAo+ID4gPiAo aWYgbm90LCB3ZSBqdXN0IHJlcGVhdCB0aGUgdGVzdCB1bnRpbCBzdGFibGUpLgo+ID4gPiAKPiA+ ID4gU2lnbmVkLW9mZi1ieTogQ2hyaXMgV2lsc29uIDxjaHJpc0BjaHJpcy13aWxzb24uY28udWs+ Cj4gPiA+IENjOiBTdW1pdCBTZW13YWwgPHN1bWl0LnNlbXdhbEBsaW5hcm8ub3JnPgo+ID4gPiBD YzogbGludXgtbWVkaWFAdmdlci5rZXJuZWwub3JnCj4gPiA+IENjOiBkcmktZGV2ZWxAbGlzdHMu ZnJlZWRlc2t0b3Aub3JnCj4gPiA+IENjOiBsaW5hcm8tbW0tc2lnQGxpc3RzLmxpbmFyby5vcmcK PiA+IAo+ID4gTm90IGVudGlyZWx5IHN1cmUgdGhpcyBpcyBzYWZlIGZvciBub24taTkxNSBkcml2 ZXJzLiBXZSBtaWdodCBub3cgY2FsbAo+ID4gLT5zaWduYWxsZWQgb24gYSB6b21iaWUgZmVuY2Ug KGkuZS4gcmVmY291bnQgPT0gMCwgYnV0IG5vdCB5ZXQga2ZyZWVkKS4KPiA+IGk5MTUgY2FuIGRv IHRoYXQsIGJ1dCBvdGhlciBkcml2ZXJzIG1pZ2h0IGdvIGJvb20uCj4gCj4gQWxsIGZlbmNlcyBt dXN0IGJlIHVuZGVyIFJDVSBndWFyZCwgb3IgaXMgdGhhdCB0aGUgc3RpY2tpbmcgcG9pbnQ/IEdp dmVuCj4gdGhhdCwgdGhlIHByb2JsZW0gaXMgZmVuY2UgcmVhbGxvY2F0aW9uIHdpdGhpbiB0aGUg UkNVIGdyYWNlIHBlcmlvZC4gSWYKPiB3ZSBjYW4gbWFuZGF0ZSB0aGF0IGZlbmNlIHJlYWxsb2Nh dGlvbiBtdXN0IGJlIHNhZmUgZm9yIGNvbmN1cnJlbnQKPiBmZW5jZS0+b3BzLT4qKCksIHdlIGNh biB1c2UgdGhpcyB0ZWNobmlxdWUgdG8gYXZvaWQgdGhlIHNlcmlhbGlzYXRpb24KPiBiYXJyaWVy IG90aGVyd2lzZSByZXF1aXJlZC4gSW4gdGhlIHNpbXBsZSBzdHJlc3MgdGVzdCwgdGhlIGRpZmZl cmVuY2UgaXMKPiBhbiBvcmRlciBvZiBtYWduaXR1ZGUsIGFuZCB0ZXN0X3NpZ25hbGVkX3JjdSBp cyBvZnRlbiBvbiBhIHBhdGggd2hlcmUKPiBldmVyeSBtZW1vcnkgYmFycmllciBxdWlja2x5IGFk ZHMgdXAgKGF0IGxlYXN0IGZvciB1cykuCj4gCj4gU28gaXMgaXQganVzdCB0aGF0IHlvdSB3b3Jy eSB0aGF0IG90aGVycyB1c2luZyBTTEFCX0RFU1RST1lfQllfUkNVIHdvbid0Cj4gZW5zdXJlIHRo ZWlyIGZlbmNlIGlzIHNhZmUgZHVyaW5nIHRoZSByZWFsbG9jYXRpb24/CgpCZWZvcmUgeW91ciBw YXRjaCB0aGUgcmN1LXByb3RlY3RlZCBwYXJ0IHdhcyBqdXN0IHRoZQprcmVmX2dldF91bmxlc3Nf emVyby4gVGhpcyB3YXMgZG9uZSBiZWZvcmUgY2FsbGluZyBkb3duIGludG8gYW5kCmZlbmMtPm9w cy0+KiBmdW5jdGlvbnMuIFdoaWNoIG1lYW5zIHRoZSBjb2RlIG9mIHRoZXNlIGZ1bmN0aW9ucyB3 YXMKZ3VhcmFudGVlZCB0byBydW4gb24gYSByZWFsIGZlbmNlIG9iamVjdCwgYW5kIG5vdCBhIHpv bWJpZSBmZW5jZSBpbiB0aGUKcHJvY2VzcyBvZiBnZXR0aW5nIGRlc3RydWN0ZWQuCgpBZmFpdWkg d2l0aCB5b3VyIHBhdGNoIHdlIG1pZ2h0IGNhbGwgaW50byBmZW5jZS0+b3BzLT4qIG9uIHRoZXNl IHpvbWJpZQpmZW5jZXMuIFRoYXQgd291bGQgYmUgYSBiZWhhdmlvdXIgY2hhbmdlIHdpdGggcmF0 aGVyIGJpZyBpbXBsaWNhdGlvbnMKKHNpbmNlIHdlJ2QgbmVlZCB0byBhdWRpdCBhbGwgZXhpc3Rp bmcgaW1wbGVtZW50YXRpb25zLCBhbmQgYWxzbyBtYWtlIHN1cmUKYWxsIGZ1dHVyZSBvbmVzIHdp bGwgYmUgb2sgdG9vKS4gT3IgSSBtaXNzZWQgc29tZXRoaW5nIGFnYWluLgoKSSB0aGluayB3ZSBj b3VsZCBzdGlsbCB0byB0aGlzIHRyaWNrLCBhdCBsZWFzdCBwYXJ0aWFsbHksIGJ5IG1ha2luZyBz dXJlCndlIG9ubHkgaW5zcGVjdCBnZW5lcmljIGZlbmNlIHN0YXRlIGFuZCBuZXZlciBjYWxsIGlu dG8gZmVuY2UtPm9wcyBiZWZvcmUKd2UncmUgZ3VhcmFudGVlZCB0byBoYXZlIGEgcmVhbCBmZW5j ZS4KCkJ1dCBhdG0gKGF0IGxlYXN0IHBlciBDaHJpc3RpYW4gS8O2bmlnKSBhIGZlbmNlIHdvbid0 IGV2ZW50dWFsbHkgZ2V0CnNpZ25hbGxlZCB3aXRob3V0IGNhbGxpbmcgaW50byAtPm9wcyBmdW5j dGlvbnMsIHNvIHRoZXJlJ3MgYSBjYXRjaCAyMi4KLURhbmllbAotLSAKRGFuaWVsIFZldHRlcgpT b2Z0d2FyZSBFbmdpbmVlciwgSW50ZWwgQ29ycG9yYXRpb24KaHR0cDovL2Jsb2cuZmZ3bGwuY2gK X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-wm0-f68.google.com ([74.125.82.68]:33443 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933812AbcIYUnN (ORCPT ); Sun, 25 Sep 2016 16:43:13 -0400 Received: by mail-wm0-f68.google.com with SMTP id w84so11218057wmg.0 for ; Sun, 25 Sep 2016 13:43:12 -0700 (PDT) Date: Sun, 25 Sep 2016 22:43:08 +0200 From: Daniel Vetter To: Chris Wilson , Daniel Vetter , 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: <20160925204308.GP20761@phenom.ffwll.local> References: <20160829070834.22296-1-chris@chris-wilson.co.uk> <20160829070834.22296-10-chris@chris-wilson.co.uk> <20160923134926.GL3988@dvetter-linux.ger.corp.intel.com> <20160923140232.GD28107@nuc-i3427.alporthouse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160923140232.GD28107@nuc-i3427.alporthouse.com> Sender: linux-media-owner@vger.kernel.org List-ID: On Fri, Sep 23, 2016 at 03:02:32PM +0100, Chris Wilson wrote: > On Fri, Sep 23, 2016 at 03:49:26PM +0200, Daniel Vetter wrote: > > 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. > > All fences must be under RCU guard, or is that the sticking point? Given > that, the problem is fence reallocation within the RCU grace period. If > we can mandate that fence reallocation must be safe for concurrent > fence->ops->*(), we can use this technique to avoid the serialisation > barrier otherwise required. In the simple stress test, the difference is > an order of magnitude, and test_signaled_rcu is often on a path where > every memory barrier quickly adds up (at least for us). > > So is it just that you worry that others using SLAB_DESTROY_BY_RCU won't > ensure their fence is safe during the reallocation? Before your patch the rcu-protected part was just the kref_get_unless_zero. This was done before calling down into and fenc->ops->* functions. Which means the code of these functions was guaranteed to run on a real fence object, and not a zombie fence in the process of getting destructed. Afaiui with your patch we might call into fence->ops->* on these zombie fences. That would be a behaviour change with rather big implications (since we'd need to audit all existing implementations, and also make sure all future ones will be ok too). Or I missed something again. I think we could still to this trick, at least partially, by making sure we only inspect generic fence state and never call into fence->ops before we're guaranteed to have a real fence. But atm (at least per Christian König) a fence won't eventually get signalled without calling into ->ops functions, so there's a catch 22. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch