From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.fireflyinternet.com ([109.228.58.192]:50391 "EHLO fireflyinternet.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756039AbdC2MPg (ORCPT ); Wed, 29 Mar 2017 08:15:36 -0400 Date: Wed, 29 Mar 2017 13:15:32 +0100 From: Chris Wilson To: Tvrtko Ursulin Cc: intel-gfx@lists.freedesktop.org, "# v4 . 10+" Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Avoid lock dropping between rescheduling Message-ID: <20170329121532.GA2909@nuc-i3427.alporthouse.com> References: <20170326084420.3231-1-chris@chris-wilson.co.uk> <20170327202143.7972-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: stable-owner@vger.kernel.org List-ID: On Wed, Mar 29, 2017 at 10:33:47AM +0100, Tvrtko Ursulin wrote: > > On 27/03/2017 21:21, Chris Wilson wrote: > >Unlocking is dangerous. In this case we combine an early update to the > >out-of-queue request, because we know that it will be inserted into the > >correct FIFO priority-ordered slot when it becomes ready in the future. > >However, given sufficient enthusiasm, it may become ready as we are > >continuing to reschedule, and so may gazump the FIFO if we have since > >dropped its spinlock. The result is that it may be executed too early, > >before its dependencies. > > > >v2: Move all work into the second phase over the topological sort. This > >removes the shortcut on the out-of-rbtree request to ensure that we only > >adjust its priority after adjusting all of its dependencies. > > > >Fixes: 20311bd35060 ("drm/i915/scheduler: Execute requests in order of priorities") > >Testcase: igt/gem_exec_whisper > >Signed-off-by: Chris Wilson > >Cc: Tvrtko Ursulin > >Cc: # v4.10+ > >--- > > drivers/gpu/drm/i915/intel_lrc.c | 44 ++++++++++++++++++---------------------- > > 1 file changed, 20 insertions(+), 24 deletions(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > >index b0c3a029b592..91e38e80a095 100644 > >--- a/drivers/gpu/drm/i915/intel_lrc.c > >+++ b/drivers/gpu/drm/i915/intel_lrc.c > >@@ -665,8 +665,8 @@ pt_lock_engine(struct i915_priotree *pt, struct intel_engine_cs *locked) > > priotree)->engine; > > if (engine != locked) { > > if (locked) > > Could replace "if (locked)" with a GEM_BUG_ON(!locked) now. > > >- spin_unlock_irq(&locked->timeline->lock); > >- spin_lock_irq(&engine->timeline->lock); > >+ spin_unlock(&locked->timeline->lock); > >+ spin_lock(&engine->timeline->lock); > > } > > > > return engine; [snip] > Looks OK to me. Preferably with the tidy in pt_lock_engine: > > Reviewed-by: Tvrtko Ursulin Pushed with the suggested improvement, thanks. -Chris -- Chris Wilson, Intel Open Source Technology Centre From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH v2] drm/i915: Avoid lock dropping between rescheduling Date: Wed, 29 Mar 2017 13:15:32 +0100 Message-ID: <20170329121532.GA2909@nuc-i3427.alporthouse.com> References: <20170326084420.3231-1-chris@chris-wilson.co.uk> <20170327202143.7972-1-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 34A596E718 for ; Wed, 29 Mar 2017 12:15:39 +0000 (UTC) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Tvrtko Ursulin Cc: intel-gfx@lists.freedesktop.org, "# v4 . 10+" List-Id: intel-gfx@lists.freedesktop.org T24gV2VkLCBNYXIgMjksIDIwMTcgYXQgMTA6MzM6NDdBTSArMDEwMCwgVHZydGtvIFVyc3VsaW4g d3JvdGU6Cj4gCj4gT24gMjcvMDMvMjAxNyAyMToyMSwgQ2hyaXMgV2lsc29uIHdyb3RlOgo+ID5V bmxvY2tpbmcgaXMgZGFuZ2Vyb3VzLiBJbiB0aGlzIGNhc2Ugd2UgY29tYmluZSBhbiBlYXJseSB1 cGRhdGUgdG8gdGhlCj4gPm91dC1vZi1xdWV1ZSByZXF1ZXN0LCBiZWNhdXNlIHdlIGtub3cgdGhh dCBpdCB3aWxsIGJlIGluc2VydGVkIGludG8gdGhlCj4gPmNvcnJlY3QgRklGTyBwcmlvcml0eS1v cmRlcmVkIHNsb3Qgd2hlbiBpdCBiZWNvbWVzIHJlYWR5IGluIHRoZSBmdXR1cmUuCj4gPkhvd2V2 ZXIsIGdpdmVuIHN1ZmZpY2llbnQgZW50aHVzaWFzbSwgaXQgbWF5IGJlY29tZSByZWFkeSBhcyB3 ZSBhcmUKPiA+Y29udGludWluZyB0byByZXNjaGVkdWxlLCBhbmQgc28gbWF5IGdhenVtcCB0aGUg RklGTyBpZiB3ZSBoYXZlIHNpbmNlCj4gPmRyb3BwZWQgaXRzIHNwaW5sb2NrLiBUaGUgcmVzdWx0 IGlzIHRoYXQgaXQgbWF5IGJlIGV4ZWN1dGVkIHRvbyBlYXJseSwKPiA+YmVmb3JlIGl0cyBkZXBl bmRlbmNpZXMuCj4gPgo+ID52MjogTW92ZSBhbGwgd29yayBpbnRvIHRoZSBzZWNvbmQgcGhhc2Ug b3ZlciB0aGUgdG9wb2xvZ2ljYWwgc29ydC4gVGhpcwo+ID5yZW1vdmVzIHRoZSBzaG9ydGN1dCBv biB0aGUgb3V0LW9mLXJidHJlZSByZXF1ZXN0IHRvIGVuc3VyZSB0aGF0IHdlIG9ubHkKPiA+YWRq dXN0IGl0cyBwcmlvcml0eSBhZnRlciBhZGp1c3RpbmcgYWxsIG9mIGl0cyBkZXBlbmRlbmNpZXMu Cj4gPgo+ID5GaXhlczogMjAzMTFiZDM1MDYwICgiZHJtL2k5MTUvc2NoZWR1bGVyOiBFeGVjdXRl IHJlcXVlc3RzIGluIG9yZGVyIG9mIHByaW9yaXRpZXMiKQo+ID5UZXN0Y2FzZTogaWd0L2dlbV9l eGVjX3doaXNwZXIKPiA+U2lnbmVkLW9mZi1ieTogQ2hyaXMgV2lsc29uIDxjaHJpc0BjaHJpcy13 aWxzb24uY28udWs+Cj4gPkNjOiBUdnJ0a28gVXJzdWxpbiA8dHZydGtvLnVyc3VsaW5AaW50ZWwu Y29tPgo+ID5DYzogPHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmc+ICMgdjQuMTArCj4gPi0tLQo+ID4g ZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfbHJjLmMgfCA0NCArKysrKysrKysrKysrKysrKyst LS0tLS0tLS0tLS0tLS0tLS0tLS0tCj4gPiAxIGZpbGUgY2hhbmdlZCwgMjAgaW5zZXJ0aW9ucygr KSwgMjQgZGVsZXRpb25zKC0pCj4gPgo+ID5kaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5 MTUvaW50ZWxfbHJjLmMgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9scmMuYwo+ID5pbmRl eCBiMGMzYTAyOWI1OTIuLjkxZTM4ZTgwYTA5NSAxMDA2NDQKPiA+LS0tIGEvZHJpdmVycy9ncHUv ZHJtL2k5MTUvaW50ZWxfbHJjLmMKPiA+KysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxf bHJjLmMKPiA+QEAgLTY2NSw4ICs2NjUsOCBAQCBwdF9sb2NrX2VuZ2luZShzdHJ1Y3QgaTkxNV9w cmlvdHJlZSAqcHQsIHN0cnVjdCBpbnRlbF9lbmdpbmVfY3MgKmxvY2tlZCkKPiA+IAkJCSAgICAg IHByaW90cmVlKS0+ZW5naW5lOwo+ID4gCWlmIChlbmdpbmUgIT0gbG9ja2VkKSB7Cj4gPiAJCWlm IChsb2NrZWQpCj4gCj4gQ291bGQgcmVwbGFjZSAiaWYgKGxvY2tlZCkiIHdpdGggYSBHRU1fQlVH X09OKCFsb2NrZWQpIG5vdy4KPiAKPiA+LQkJCXNwaW5fdW5sb2NrX2lycSgmbG9ja2VkLT50aW1l bGluZS0+bG9jayk7Cj4gPi0JCXNwaW5fbG9ja19pcnEoJmVuZ2luZS0+dGltZWxpbmUtPmxvY2sp Owo+ID4rCQkJc3Bpbl91bmxvY2soJmxvY2tlZC0+dGltZWxpbmUtPmxvY2spOwo+ID4rCQlzcGlu X2xvY2soJmVuZ2luZS0+dGltZWxpbmUtPmxvY2spOwo+ID4gCX0KPiA+Cj4gPiAJcmV0dXJuIGVu Z2luZTsKCltzbmlwXQoKPiBMb29rcyBPSyB0byBtZS4gUHJlZmVyYWJseSB3aXRoIHRoZSB0aWR5 IGluIHB0X2xvY2tfZW5naW5lOgo+IAo+IFJldmlld2VkLWJ5OiBUdnJ0a28gVXJzdWxpbiA8dHZy dGtvLnVyc3VsaW5AaW50ZWwuY29tPgoKUHVzaGVkIHdpdGggdGhlIHN1Z2dlc3RlZCBpbXByb3Zl bWVudCwgdGhhbmtzLgotQ2hyaXMKCi0tIApDaHJpcyBXaWxzb24sIEludGVsIE9wZW4gU291cmNl IFRlY2hub2xvZ3kgQ2VudHJlCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNr dG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2lu dGVsLWdmeAo=