From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH] drm/i915: Unconditionally flush writes before execbuffer Date: Thu, 21 May 2015 13:29:25 -0700 Message-ID: <555E4025.5060403@virtuousgeek.org> References: <1431330696-15379-1-git-send-email-chris@chris-wilson.co.uk> <20150511103437.GA15256@phenom.ffwll.local> <20150511152552.GD19022@nuc-i3427.alporthouse.com> <20150519144148.GA13637@nuc-i3427.alporthouse.com> <20150521130034.GN17761@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from gproxy9-pub.mail.unifiedlayer.com (gproxy9-pub.mail.unifiedlayer.com [69.89.20.122]) by gabe.freedesktop.org (Postfix) with SMTP id 73C7E6E13D for ; Thu, 21 May 2015 13:29:53 -0700 (PDT) In-Reply-To: <20150521130034.GN17761@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Daniel Vetter , intel-gfx@lists.freedesktop.org, Akash Goel , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gMDUvMjEvMjAxNSAwNjowMCBBTSwgQ2hyaXMgV2lsc29uIHdyb3RlOgo+IE9uIFR1ZSwgTWF5 IDE5LCAyMDE1IGF0IDAzOjQxOjQ4UE0gKzAxMDAsIENocmlzIFdpbHNvbiB3cm90ZToKPj4gT24g TW9uLCBNYXkgMTEsIDIwMTUgYXQgMDQ6MjU6NTJQTSArMDEwMCwgQ2hyaXMgV2lsc29uIHdyb3Rl Ogo+Pj4gT24gTW9uLCBNYXkgMTEsIDIwMTUgYXQgMTI6MzQ6MzdQTSArMDIwMCwgRGFuaWVsIFZl dHRlciB3cm90ZToKPj4+PiBPbiBNb24sIE1heSAxMSwgMjAxNSBhdCAwODo1MTozNkFNICswMTAw LCBDaHJpcyBXaWxzb24gd3JvdGU6Cj4+Pj4+IFdpdGggdGhlIGFkdmVudCBvZiBtbWFwKHdjKSwg d2UgaGF2ZSBhIHBhdGggdG8gd3JpdGUgZGlyZWN0bHkgaW50bwo+Pj4+PiBhY3RpdmUgR1BVIGJ1 ZmZlcnMuIFdoZW4gY29tYmluZWQgd2l0aCBhc3luYyB1cGRhdGVzIChpLmUuIGF2b2lkaW5nIHRo ZQo+Pj4+PiBleHBsaWNpdCBkb21haW4gbWFuYWdlbWVudCBhbG9uZyB3aXRoIHRoZSBtZW1vcnkg YmFycmllcnMgYW5kIEdQVQo+Pj4+PiBzdGFsbHMpIHdlIHN0YXJ0IHRvIHNlZSB0aGUgR1BVIHJl YWQgdGhlIHdyb25nIHZhbHVlcyBmcm9tIG1lbW9yeSAtIGkuZS4KPj4+Pj4gd2UgaGF2ZSBpbnN1 ZmZpY2llbnQgbWVtb3J5IGJhcnJpZXJzIGFsb25nIHRoZSBleGVjYnVmZmVyIHBhdGguIFdyaXRl cwo+Pj4+PiB0aHJvdWdoIHRoZSBHVFQgc2hvdWxkIGhhdmUgYmVlbiBuYXR1cmFsbHkgc2VyaWFs aXNlZCB3aXRoIGV4ZWN1dGlvbgo+Pj4+PiB0aHJvdWdoIHRoZSBHVFQgYXMgd2VsbCBhbmQgc28g dGhlIGltcGFjdCBvbmx5IHNlZW1zIHRvIGJlIGZyb20gdGhlIFdDCj4+Pj4+IHBhdGhzLgo+Pj4+ Pgo+Pj4+PiBTaWduZWQtb2ZmLWJ5OiBDaHJpcyBXaWxzb24gPGNocmlzQGNocmlzLXdpbHNvbi5j by51az4KPj4+Pj4gQ2M6IEFrYXNoIEdvZWwgPGFrYXNoLmdvZWxAaW50ZWwuY29tPgo+Pj4+PiBD Yzogc3RhYmxlQHZnZXIua2VybmVsLm9yZwo+Pj4+Cj4+Pj4gRG8gd2UgaGF2ZSBhIG5hc3R5IGln dCBmb3IgdGhpcz8gQnVnemlsbGE/Cj4+Pgo+Pj4gSSd2ZSBhZGRlZCBpZ3QvZ2VtX3N0cmVhbWlu Z193cml0ZXMuCj4+Pgo+Pj4gVGhhdCB3bWIoKSBpcyBub3QgZW5vdWdoIGZvciAhbGxjLiBTaW5j ZSB0aGUgd21iKCkgbWFkZSBwaWdsaXQgaGFwcHkgaXQKPj4+IGlzIHF1aXRlIHBvc3NpYmxlIEkg aGF2ZW4ndCBoaXQgdGhlIHNhbWUgcGF0aCBleGFjdGx5LCBidXQgaXQncyBnb2luZyB0bwo+Pj4g dGFrZSBzb21lIGludmVzdGlnYXRpb24gdG8gc2VlIGlmIGlndC9nZW1fc3RyZWFtaW5nX3dyaXRl cyBjYW4gcG9zc2libHkKPj4+IHdvcmsgb24gIWxsYy4KPj4KPj4gSHVtYnVnLgo+Pgo+PiBGb3Vu ZCB0aGUgYnVnIGluIGdlbV9zdHJlYW1pbmdfd3JpdGVzLCBldmVuIHRob3VnaCBJIHN0aWxsIHRo aW5rIHRoZQo+PiB3bWIoKSBpcyBzdHJpY3RseSByZXF1aXJlZCwgaXQgcnVucyBmaW5lIHdpdGhv dXQgKHByZXN1bWFibHkgSSBoYXZlbid0Cj4+IG1hbmFnZWQgdG8gYXZvaWQgYWxsIGJhcnJpZXJz IGluIHRoZSBleGVjYnVmZmVyIHBhdGggeWV0KS4gSG93ZXZlciwgSQo+PiB0aGluayBjYW4gaW1w cm92ZSB0aGUgc3RyZXNzIGJ5IGluc2VydGluZyBleHRyYSBncHUgbG9hZCAtLSB0aGF0IHNob3Vs ZAo+PiBoZWxwIG1ha2UgdGhlIENQVSB3cml0ZXMgLyBHUFUgcmVhZHMgb2YgdGhlIGJ1ZmZlciBj b25jdXJyZW50Pwo+IAo+IEp1c3QgYSBzbWFsbCB1cGRhdGUuIEkgaGF2ZW4ndCBmb3VuZCBhIHdh eSB0byByZXByb2R1Y2UgdGhpcyBpbiBpZ3QgeWV0LAo+IGJ1dCBJIGNhbiBzdGlsbCBvYnNlcnZl IHRoZSBlZmZlY3QgdXNpbmcgdmJvLW1hcC11bnN5bmMgYW5kIHRoZSBmaXgKPiB0aGVyZSBpcyB0 aGUgYWJvdmUgcGF0Y2ggdG8gbWFrZSB0aGUgd21iKCkgdW5jb25kaXRpb25hbC4KPiAKPiBXZSBu ZWVkIHRvIHB1dCB0aGlzIGludG8gc3RhYmxlQCByZWFzb25hYmx5IHF1aWNrbHkgKEkgc3VzcGVj dCBzb21lIG9mCj4gdGhlIDQuMCBtbWFwKHdjKSByZWdyZXNzaW9ucyBhcmUgZHVlIHRvIHRoaXMg YXMgd2VsbCkuCgpTbyB0aGUgc3ltcHRvbSBpcyB0aGF0IHRoZSBHUFUgcGlja3MgdXAgb2xkZXIg dmFsdWVzIGZyb20gbWVtb3J5LCBhbmQKeW91ciB0aGVvcnkgaXMgdGhhdCB0aGUgd21iKCkga2lj a3MgdGhlIHZhbHVlcyBvdXQgb2YgdGhlIHN0b3JlIGJ1ZmZlcgpvciBXQyBidWZmZXIgcHJpb3Ig dG8gdGhlIGV4ZWNidWY/CgpJIHRoaW5rIHRoYXQncyByZWFzb25hYmxlLCBhbmQgSSdtIGhvcGlu ZyAiZ2xvYmFsbHkgdmlzaWJsZSIgaXMgc3BlYydkCnRvIGluY2x1ZGUgdGhlIEdQVSBhbmQgb3Ro ZXIgc3lzdGVtIGFnZW50cyBpbiB0aGUgc2ZlbmNlIGRlZmluaXRpb24uCgpKZXNzZQoKX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxp bmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVl ZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gproxy1-pub.mail.unifiedlayer.com ([69.89.25.95]:48420 "HELO gproxy1-pub.mail.unifiedlayer.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1756114AbbEUUg3 (ORCPT ); Thu, 21 May 2015 16:36:29 -0400 Message-ID: <555E4025.5060403@virtuousgeek.org> Date: Thu, 21 May 2015 13:29:25 -0700 From: Jesse Barnes MIME-Version: 1.0 To: Chris Wilson , Daniel Vetter , intel-gfx@lists.freedesktop.org, Akash Goel , stable@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Unconditionally flush writes before execbuffer References: <1431330696-15379-1-git-send-email-chris@chris-wilson.co.uk> <20150511103437.GA15256@phenom.ffwll.local> <20150511152552.GD19022@nuc-i3427.alporthouse.com> <20150519144148.GA13637@nuc-i3427.alporthouse.com> <20150521130034.GN17761@nuc-i3427.alporthouse.com> In-Reply-To: <20150521130034.GN17761@nuc-i3427.alporthouse.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 05/21/2015 06:00 AM, Chris Wilson wrote: > On Tue, May 19, 2015 at 03:41:48PM +0100, Chris Wilson wrote: >> On Mon, May 11, 2015 at 04:25:52PM +0100, Chris Wilson wrote: >>> On Mon, May 11, 2015 at 12:34:37PM +0200, Daniel Vetter wrote: >>>> On Mon, May 11, 2015 at 08:51:36AM +0100, Chris Wilson wrote: >>>>> With the advent of mmap(wc), we have a path to write directly into >>>>> active GPU buffers. When combined with async updates (i.e. avoiding the >>>>> explicit domain management along with the memory barriers and GPU >>>>> stalls) we start to see the GPU read the wrong values from memory - i.e. >>>>> we have insufficient memory barriers along the execbuffer path. Writes >>>>> through the GTT should have been naturally serialised with execution >>>>> through the GTT as well and so the impact only seems to be from the WC >>>>> paths. >>>>> >>>>> Signed-off-by: Chris Wilson >>>>> Cc: Akash Goel >>>>> Cc: stable@vger.kernel.org >>>> >>>> Do we have a nasty igt for this? Bugzilla? >>> >>> I've added igt/gem_streaming_writes. >>> >>> That wmb() is not enough for !llc. Since the wmb() made piglit happy it >>> is quite possible I haven't hit the same path exactly, but it's going to >>> take some investigation to see if igt/gem_streaming_writes can possibly >>> work on !llc. >> >> Humbug. >> >> Found the bug in gem_streaming_writes, even though I still think the >> wmb() is strictly required, it runs fine without (presumably I haven't >> managed to avoid all barriers in the execbuffer path yet). However, I >> think can improve the stress by inserting extra gpu load -- that should >> help make the CPU writes / GPU reads of the buffer concurrent? > > Just a small update. I haven't found a way to reproduce this in igt yet, > but I can still observe the effect using vbo-map-unsync and the fix > there is the above patch to make the wmb() unconditional. > > We need to put this into stable@ reasonably quickly (I suspect some of > the 4.0 mmap(wc) regressions are due to this as well). So the symptom is that the GPU picks up older values from memory, and your theory is that the wmb() kicks the values out of the store buffer or WC buffer prior to the execbuf? I think that's reasonable, and I'm hoping "globally visible" is spec'd to include the GPU and other system agents in the sfence definition. Jesse