From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH 1/2] drm: refernce count event->completion Date: Thu, 09 Feb 2017 16:39:29 +0200 Message-ID: <87mvdvsa1q.fsf@intel.com> References: <20161221102331.31033-1-daniel.vetter@ffwll.ch> <20161221103641.GA735@nuc-i3427.alporthouse.com> <5d8242c1-f3e5-4042-a222-c36119089e68@linux.intel.com> <20170104100510.fhgu237qfhiv47gr@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20170104100510.fhgu237qfhiv47gr@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter , Maarten Lankhorst Cc: Daniel Vetter , Intel Graphics Development , Jim Rees , stable@vger.kernel.org, DRI Development , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gV2VkLCAwNCBKYW4gMjAxNywgRGFuaWVsIFZldHRlciA8ZGFuaWVsQGZmd2xsLmNoPiB3cm90 ZToKPiBPbiBXZWQsIERlYyAyMSwgMjAxNiBhdCAxMjowODo0NVBNICswMTAwLCBNYWFydGVuIExh bmtob3JzdCB3cm90ZToKPj4gT3AgMjEtMTItMTYgb20gMTE6MzYgc2NocmVlZiBDaHJpcyBXaWxz b246Cj4+ID4gT24gV2VkLCBEZWMgMjEsIDIwMTYgYXQgMTE6MjM6MzBBTSArMDEwMCwgRGFuaWVs IFZldHRlciB3cm90ZToKPj4gPj4gV2hlbiB3cml0aW5nIHRoZSBnZW5lcmljIG5vbmJsb2NraW5n IGNvbW1pdCBjb2RlIEkgYXNzdW1lZCB0aGF0Cj4+ID4+IHRocm91Z2ggY2xldmVyIGxpZmV0aW1l IG1hbmFnZW1lbnQgSSBjYW4gYXNzdXJlIHRoYXQgdGhlIGNvbXBsZXRpb24KPj4gPj4gKHN0b3Jl ZCBpbiBkcm1fY3J0Y19jb21taXQpIG9ubHkgZ2V0cyBmcmVlZCBhZnRlciBpdCBpcyBjb21wbGV0 ZWQuIEFuZAo+PiA+PiB0aGF0IHdvcmtlZC4KPj4gPj4KPj4gPj4gSSBhbHNvIHdhbnRlZCB0byBt YWtlIG5vbmJsb2NraW5nIGhlbHBlcnMgcmVzaWxpZW50IGFnYWluc3QgZHJpdmVyCj4+ID4+IGJ1 Z3MsIGJ5IGhhdmluZyB0aW1lb3V0cyBldmVyeXdoZXJlLiBBbmQgdGhhdCB3b3JrZWQgdG9vLgo+ PiA+Pgo+PiA+PiBVbmZvcnR1bmF0ZWx5IHRha2luZyBib3RocyB0aGluZ3MgdG9nZXRoZXIgcmVz dWx0cyBpbiBvb3BzZXMgOiggV2VsbCwKPj4gPj4gYXQgbGVhc3Qgc29tZXRpbWVzOiBXaGF0IHNl ZW1zIHRvIGhhcHBlbiBpcyB0aGF0IHRoZSBkcm0gZXZlbnQgaGFuZ3MKPj4gPj4gYXJvdW5kIGZv cmV2ZXIgc3R1Y2sgaW4gbGltYm8gbGFuZC4gVGhlIG5vbmJsb2NraW5nIGhlbHBlcnMgZXZlbnR1 YWxseQo+PiA+PiB0aW1lIG91dCwgbW92ZSBvbiBhbmQgcmVsZWFzZSBpdC4gTm93IHRoZSBidWcg SSB0ZXN0ZWQgYWxsIHRoaXMKPj4gPj4gYWdhaW5zdCBpcyBkcml2ZXJzIHRoYXQganVzdCBlbnRp cmVseSBmYWlsIHRvIGRlbGl2ZXIgdGhlIHZibGFuawo+PiA+PiBldmVudHMgbGlrZSB0aGV5IHNo b3VsZCwgYW5kIGluIHRob3NlIGNhc2VzIHRoZSBldmVudCBpcyBzaW1wbHkKPj4gPj4gbGVha2Vk LiBCdXQgd2hhdCBzZWVtcyB0byBoYXBwZW4sIGF0IGxlYXN0IHNvbWV0aW1lcywgb24gaTkxNSBp cyB0aGF0Cj4+ID4+IHRoZSBldmVudCBpcyBzZXQgdXAgY29ycmVjdGx5LCBidXQgc29tb2hvdyB0 aGUgdmJsYW5rIGZhaWxzIHRvIGZpcmUgaW4KPj4gPj4gdGltZS4gV2hpY2ggbWVhbnMgdGhlIGV2 ZW50IGlzbid0IGxlYWtlZCwgaXQncyBzdGlsbCB0aGVyZSB3YWl0aW5nIGZvcgo+PiA+PiBldmV2 bnR1YWxseSBhIHZibGFuayB0byBmaXJlLiBUaGF0IHRlbmRzIHRvIGhhcHBlbiB3aGVuIHJlLWVu YWJsaW5nIHRoZQo+PiA+PiBwaXBlLCBhbmQgdGhlbiB0aGUgdHJhcCBzcHJpbmdzIGFuZCB0aGUg a2VybmVsIG9vcHNlcy4KPj4gPj4KPj4gPj4gVGhlIGNvcnJlY3QgZml4IGhlcmUgaXMgc2ltcGx5 IHRvIHJlZmNvdW50IHRoZSBjcnRjIGNvbW1pdCB0byBtYWtlCj4+ID4+IHN1cmUgdGhhdCB0aGUg ZXZlbnQgc3RpY2tzIGFyb3VuZCBldmVuIGZvciBkcml2ZXJzIHdoaWNoIG9ubHkKPj4gPj4gc29t ZXRpbWVzIGZhaWwgdG8gZGVsaXZlciB2YmxhbmtzIGZvciBzb21lIGFyYml0cmFyeSByZWFzb25z LiBTaW5jZQo+PiA+PiBjcnRjIGNvbW1pdHMgYXJlIGFscmVhZHkgcmVmY291bnRlZCB0aGF0J3Mg ZWFzeSB0byBkby4KPj4gPiBPciBtYWtlIHRoZSBldmVudCBhIHBhcnQgb2YgdGhlIGF0b21pYyBz dGF0ZT8KPj4gPiAtQ2hyaXMKPj4gPgo+PiBhZmFpY3QgY3J0YyBjb21taXQgaXMgYWxyZWFkeSB0 YWtlbiB0byB3YWl0IGZvciBjb21wbGV0aW9uLCBzbyB0aGlzIHBhdGNoIG1ha2VzIHNlbnNlLgo+ PiAKPj4gVGhlcmUncyBqdXN0IGEgbWlub3IgdHlwbyBpbiB0aGUgc3ViamVjdC4gOikKPj4gTm90 IHN1cmUgdGhhdCByZWxlYXNlX2NvbW1pdCBzaG91bGQgYmUgYSBmdW5jdGlvbiBwb2ludGVyLCBy ZWdhcmRsZXNzLi4KPj4gCj4+IFJldmlld2VkLWJ5OiBNYWFydGVuIExhbmtob3JzdCA8bWFhcnRl bi5sYW5raG9yc3RAbGludXguaW50ZWwuY29tPgo+Cj4gSXQgZGlkbid0IGhlbHAgdGhlIGJ1ZyBy ZXBvcnRlcnMgYWdhaW5zdCBvb3BzZXMgKGJ1dCB0aGUgcmVwb3J0ZXJzIGFyZQo+IHN1cHJlbWVs eSBjb25mdXNpbmcsIEkgaGF2ZSBubyBpZGVhIHdoYXQncyByZWFsbHkgYmVpbmcgdGVzdGVkLCB0 aGUKPiBidWd6aWxsYSBpcyBhIG1lc3MpLCBidXQgSSBzdGlsbCB0aGluayB0aGUgcGF0Y2ggaXMg dXNlZnVsIGZvciBtb3JlCj4gcm9idWVzdG5lc3MsIEkgZHJvcHBlZCB0aGUgY2M6IHN0YWJsZSBh bmQgYXBwbGllZCBpdCB0byBkcm0tbWlzYy4KCkFncmVlZCBvbiB0aGUgYnVnIFsxXSBiZWluZyBh IG1lc3MuIEhvd2V2ZXIsIHRoZSBidWcgaGFzIGEgcmVsaWFibGUKYmlzZWN0IHJlc3VsdCwgdGhl IHJldmVydCB3YXMgcG9zdGVkIGJ5IHNvbWUgb2YgdGhlIHJlcG9ydGVycyBvbiB0aGUKbGlzdHMg YW5kIGluIHRoZSBidWcsIGFuZCBub3cgc29tZXRoaW5nIHRoYXQgd2lsbCBub3QgaGVscCBhbnlv bmUgaW4KdjQuOSBvciB2NC4xMCB3YXMgcHVzaGVkLiA6KAoKQlIsCkphbmkuCgoKWzFdIGh0dHBz Oi8vYnVncy5mcmVlZGVza3RvcC5vcmcvc2hvd19idWcuY2dpP2lkPTk2NzgxCgotLSAKSmFuaSBO aWt1bGEsIEludGVsIE9wZW4gU291cmNlIFRlY2hub2xvZ3kgQ2VudGVyCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QK ZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9w Lm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com ([192.55.52.43]:64407 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752282AbdBIOkr (ORCPT ); Thu, 9 Feb 2017 09:40:47 -0500 From: Jani Nikula To: Daniel Vetter , Maarten Lankhorst Cc: Chris Wilson , Daniel Vetter , DRI Development , Intel Graphics Development , Jim Rees , stable@vger.kernel.org, Daniel Vetter Subject: Re: [PATCH 1/2] drm: refernce count event->completion In-Reply-To: <20170104100510.fhgu237qfhiv47gr@phenom.ffwll.local> References: <20161221102331.31033-1-daniel.vetter@ffwll.ch> <20161221103641.GA735@nuc-i3427.alporthouse.com> <5d8242c1-f3e5-4042-a222-c36119089e68@linux.intel.com> <20170104100510.fhgu237qfhiv47gr@phenom.ffwll.local> Date: Thu, 09 Feb 2017 16:39:29 +0200 Message-ID: <87mvdvsa1q.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: stable-owner@vger.kernel.org List-ID: On Wed, 04 Jan 2017, Daniel Vetter wrote: > On Wed, Dec 21, 2016 at 12:08:45PM +0100, Maarten Lankhorst wrote: >> Op 21-12-16 om 11:36 schreef Chris Wilson: >> > On Wed, Dec 21, 2016 at 11:23:30AM +0100, Daniel Vetter wrote: >> >> When writing the generic nonblocking commit code I assumed that >> >> through clever lifetime management I can assure that the completion >> >> (stored in drm_crtc_commit) only gets freed after it is completed. And >> >> that worked. >> >> >> >> I also wanted to make nonblocking helpers resilient against driver >> >> bugs, by having timeouts everywhere. And that worked too. >> >> >> >> Unfortunately taking boths things together results in oopses :( Well, >> >> at least sometimes: What seems to happen is that the drm event hangs >> >> around forever stuck in limbo land. The nonblocking helpers eventually >> >> time out, move on and release it. Now the bug I tested all this >> >> against is drivers that just entirely fail to deliver the vblank >> >> events like they should, and in those cases the event is simply >> >> leaked. But what seems to happen, at least sometimes, on i915 is that >> >> the event is set up correctly, but somohow the vblank fails to fire in >> >> time. Which means the event isn't leaked, it's still there waiting for >> >> evevntually a vblank to fire. That tends to happen when re-enabling the >> >> pipe, and then the trap springs and the kernel oopses. >> >> >> >> The correct fix here is simply to refcount the crtc commit to make >> >> sure that the event sticks around even for drivers which only >> >> sometimes fail to deliver vblanks for some arbitrary reasons. Since >> >> crtc commits are already refcounted that's easy to do. >> > Or make the event a part of the atomic state? >> > -Chris >> > >> afaict crtc commit is already taken to wait for completion, so this patch makes sense. >> >> There's just a minor typo in the subject. :) >> Not sure that release_commit should be a function pointer, regardless.. >> >> Reviewed-by: Maarten Lankhorst > > It didn't help the bug reporters against oopses (but the reporters are > supremely confusing, I have no idea what's really being tested, the > bugzilla is a mess), but I still think the patch is useful for more > robuestness, I dropped the cc: stable and applied it to drm-misc. Agreed on the bug [1] being a mess. However, the bug has a reliable bisect result, the revert was posted by some of the reporters on the lists and in the bug, and now something that will not help anyone in v4.9 or v4.10 was pushed. :( BR, Jani. [1] https://bugs.freedesktop.org/show_bug.cgi?id=96781 -- Jani Nikula, Intel Open Source Technology Center