From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gustavo Padovan Subject: Re: [PATCH] android: fix warning when releasing active sync point Date: Tue, 15 Dec 2015 17:00:08 -0200 Message-ID: <20151215190008.GE883@joana> References: <20151215012955.GA28277@dtor-ws> <20151215092601.GI3189@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-qk0-f170.google.com (mail-qk0-f170.google.com [209.85.220.170]) by gabe.freedesktop.org (Postfix) with ESMTPS id 1891E6E776 for ; Tue, 15 Dec 2015 11:00:18 -0800 (PST) Received: by mail-qk0-f170.google.com with SMTP id t125so28279615qkh.3 for ; Tue, 15 Dec 2015 11:00:18 -0800 (PST) Content-Disposition: inline In-Reply-To: <20151215092601.GI3189@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: Dmitry Torokhov , Greg Kroah-Hartman , devel@driverdev.osuosl.org, Andrew Bresticker , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Riley Andrews , linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org MjAxNS0xMi0xNSBEYW5pZWwgVmV0dGVyIDxkYW5pZWxAZmZ3bGwuY2g+OgoKPiBPbiBNb24sIERl YyAxNCwgMjAxNSBhdCAwNToyOTo1NVBNIC0wODAwLCBEbWl0cnkgVG9yb2tob3Ygd3JvdGU6Cj4g PiBVc2Vyc3BhY2UgY2FuIGNsb3NlIHRoZSBzeW5jIGRldmljZSB3aGlsZSB0aGVyZSBhcmUgc3Rp bGwgYWN0aXZlIGZlbmNlCj4gPiBwb2ludHMsIGluIHdoaWNoIGNhc2Uga2VybmVsIHByb2R1Y2Vz IHRoZSBmb2xsb3dpbmcgd2FybmluZzoKPiA+IAo+ID4gWyAgIDQzLjg1MzE3Nl0gLS0tLS0tLS0t LS0tWyBjdXQgaGVyZSBdLS0tLS0tLS0tLS0tCj4gPiBbICAgNDMuODU3ODM0XSBXQVJOSU5HOiBD UFU6IDAgUElEOiA4OTIgYXQgL21udC9ob3N0L3NvdXJjZS9zcmMvdGhpcmRfcGFydHkva2VybmVs L3YzLjE4L2RyaXZlcnMvc3RhZ2luZy9hbmRyb2lkL3N5bmMuYzo0MzkgYW5kcm9pZF9mZW5jZV9y ZWxlYXNlKzB4ODgvMHgxMDQoKQo+ID4gWyAgIDQzLjg3MTc0MV0gQ1BVOiAwIFBJRDogODkyIENv bW06IEJpbmRlcl81IFRhaW50ZWQ6IEcgICAgIFUgMy4xOC4wLTA3NjYxLWcwNTUwY2U5ICMxCj4g PiBbICAgNDMuODgwMTc2XSBIYXJkd2FyZSBuYW1lOiBHb29nbGUgVGVncmEyMTAgU21hdWcgUmV2 IDErIChEVCkKPiA+IFsgICA0My44ODU4MzRdIENhbGwgdHJhY2U6Cj4gPiBbICAgNDMuODg4Mjk0 XSBbPGZmZmZmZmMwMDAyMDc0NjQ+XSBkdW1wX2JhY2t0cmFjZSsweDAvMHgxMGMKPiA+IFsgICA0 My44OTM2OTddIFs8ZmZmZmZmYzAwMDIwNzU4MD5dIHNob3dfc3RhY2srMHgxMC8weDFjCj4gPiBb ICAgNDMuODk4NzU2XSBbPGZmZmZmZmMwMDBhYjEyNTg+XSBkdW1wX3N0YWNrKzB4NzQvMHhiOAo+ ID4gWyAgIDQzLjkwMzgxNF0gWzxmZmZmZmZjMDAwMjFkNDE0Pl0gd2Fybl9zbG93cGF0aF9jb21t b24rMHg4NC8weGIwCj4gPiBbICAgNDMuOTA5NzM2XSBbPGZmZmZmZmMwMDAyMWQ1MzA+XSB3YXJu X3Nsb3dwYXRoX251bGwrMHgxNC8weDIwCj4gPiBbICAgNDMuOTE1NDgyXSBbPGZmZmZmZmMwMDA4 OGFlZmM+XSBhbmRyb2lkX2ZlbmNlX3JlbGVhc2UrMHg4NC8weDEwNAo+ID4gWyAgIDQzLjkyMTU4 Ml0gWzxmZmZmZmZjMDAwNjcxY2M0Pl0gZmVuY2VfcmVsZWFzZSsweDEwNC8weDEzNAo+ID4gWyAg IDQzLjkyNzA2Nl0gWzxmZmZmZmZjMDAwODhiMGNjPl0gc3luY19mZW5jZV9mcmVlKzB4NzQvMHg5 Ywo+ID4gWyAgIDQzLjkzMjU1Ml0gWzxmZmZmZmZjMDAwODhiMTI4Pl0gc3luY19mZW5jZV9yZWxl YXNlKzB4MzQvMHg0OAo+ID4gWyAgIDQzLjkzODMwNF0gWzxmZmZmZmZjMDAwMzE3YmJjPl0gX19m cHV0KzB4MTAwLzB4MWI4Cj4gPiBbICAgNDMuOTQzMTg1XSBbPGZmZmZmZmMwMDAzMTdjYzg+XSBf X19fZnB1dCsweDgvMHgxNAo+ID4gWyAgIDQzLjk0Nzk4Ml0gWzxmZmZmZmZjMDAwMjM3ZjM4Pl0g dGFza193b3JrX3J1bisweGIwLzB4ZTQKPiA+IFsgICA0My45NTMyOTddIFs8ZmZmZmZmYzAwMDIw NzA3ND5dIGRvX25vdGlmeV9yZXN1bWUrMHg0NC8weDVjCj4gPiBbICAgNDMuOTU4ODY3XSAtLS1b IGVuZCB0cmFjZSA1YTJhYTQwMjdjYzVkMTcxIF0tLS0KPiA+IAo+ID4gTGV0J3MgZml4IGl0IGJ5 IGludHJvZHVjaW5nIGEgbmV3IG9wdGlvbmFsIGNhbGxiYWNrIChkaXNhYmxlX3NpZ25hbGluZykK PiA+IHRvIGZlbmNlIG9wZXJhdGlvbnMgc28gdGhhdCBkcml2ZXJzIGNhbiBkbyBwcm9wZXIgY2xl YW4gdXBzIHdoZW4gd2UKPiA+IHJlbW92ZSBsYXN0IGNhbGxiYWNrIGZvciBnaXZlbiBmZW5jZS4K PiA+IAo+ID4gUmV2aWV3ZWQtYnk6IEFuZHJldyBCcmVzdGlja2VyIDxhYnJlc3RpY0BjaHJvbWl1 bS5vcmc+Cj4gPiBTaWduZWQtb2ZmLWJ5OiBEbWl0cnkgVG9yb2tob3YgPGR0b3JAY2hyb21pdW0u b3JnPgo+ID4gLS0tCj4gPiAgZHJpdmVycy9kbWEtYnVmL2ZlbmNlLmMgICAgICAgIHwgNiArKysr Ky0KPiA+ICBkcml2ZXJzL3N0YWdpbmcvYW5kcm9pZC9zeW5jLmMgfCA4ICsrKysrKysrCj4gPiAg aW5jbHVkZS9saW51eC9mZW5jZS5oICAgICAgICAgIHwgMiArKwo+ID4gIDMgZmlsZXMgY2hhbmdl ZCwgMTUgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQo+ID4gCj4gPiBkaWZmIC0tZ2l0IGEv ZHJpdmVycy9kbWEtYnVmL2ZlbmNlLmMgYi9kcml2ZXJzL2RtYS1idWYvZmVuY2UuYwo+ID4gaW5k ZXggN2IwNWRiZS4uMGVkNzNhZCAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMvZG1hLWJ1Zi9mZW5j ZS5jCj4gPiArKysgYi9kcml2ZXJzL2RtYS1idWYvZmVuY2UuYwo+ID4gQEAgLTMwNCw4ICszMDQs MTIgQEAgZmVuY2VfcmVtb3ZlX2NhbGxiYWNrKHN0cnVjdCBmZW5jZSAqZmVuY2UsIHN0cnVjdCBm ZW5jZV9jYiAqY2IpCj4gPiAgCXNwaW5fbG9ja19pcnFzYXZlKGZlbmNlLT5sb2NrLCBmbGFncyk7 Cj4gPiAgCj4gPiAgCXJldCA9ICFsaXN0X2VtcHR5KCZjYi0+bm9kZSk7Cj4gPiAtCWlmIChyZXQp Cj4gPiArCWlmIChyZXQpIHsKPiA+ICAJCWxpc3RfZGVsX2luaXQoJmNiLT5ub2RlKTsKPiA+ICsJ CWlmIChsaXN0X2VtcHR5KCZmZW5jZS0+Y2JfbGlzdCkpCj4gPiArCQkJaWYgKGZlbmNlLT5vcHMt PmRpc2FibGVfc2lnbmFsaW5nKQo+ID4gKwkJCQlmZW5jZS0+b3BzLT5kaXNhYmxlX3NpZ25hbGlu ZyhmZW5jZSk7Cj4gCj4gV2hhdCBleGFjdGx5IGlzIHRoZSBidWcgaGVyZT8gQSBmZW5jZSB3aXRo IG5vIGNhbGxiYWNrcyByZWdpc3RlcmVkIGFueQo+IG1vcmUgc2hvdWxkbid0IGhhdmUgYW55IHBy b2JsZW0uIFdoeSBleGFjdGx5IGRvZXMgdGhpcyBibG93IHVwPwoKVGhlIFdBUk5fT04gaXMgcHJv YmFibHkgdGhpcyBvbmU6Cmh0dHBzOi8vYW5kcm9pZC5nb29nbGVzb3VyY2UuY29tL2tlcm5lbC9j b21tb24vKy9hbmRyb2lkLTMuMTgvZHJpdmVycy9zdGFnaW5nL2FuZHJvaWQvc3luYy5jIzQzMwoK SSd2ZSBiZWVuIHdvbmRlcmluZyBpbiB0aGUgbGFzdCBmZXcgZGF5cyBpZiB0aGlzIHdhcm5pbmcg aXMgcmVhbGx5Cm5lY2Vzc2FyeS4gSWYgdGhlIHVzZXIgaXMgY2xvc2luZyBhIHN5bmNfdGltZWxp bmUgdGhhdCBoYXMgdW5zaWduYWxsZWQKZmVuY2VzIGl0IHNob3VsZCBwcm9iYWJseSBiZSBhd2Fy ZSBvZiB0aGF0IGFscmVhZHkuIFRoZW4gSSB0aGluayBpdCBpcwpva2F5IHRvIHJlbW92ZSB0aGUg dGhlIHN5bmNfcHQgZnJvbSB0aGUgYWN0aXZlX2xpc3QgYXQgdGhlIHJlbGVhc2UtdGltZS4KSW4g ZmFjdCBJJ3ZlIGFscmVhZHkgcHJlcGFyZWQgYSBwYXRjaCBkb2luZyB0aGF0LiBUaG91Z2h0cz8g IAoKCUd1c3Rhdm8KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3Jn Cmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-qk0-f170.google.com ([209.85.220.170]:35897 "EHLO mail-qk0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932232AbbLOTAS (ORCPT ); Tue, 15 Dec 2015 14:00:18 -0500 Date: Tue, 15 Dec 2015 17:00:08 -0200 From: Gustavo Padovan To: Dmitry Torokhov , Greg Kroah-Hartman , devel@driverdev.osuosl.org, Andrew Bresticker , Arve =?iso-8859-1?B?SGr4bm5lduVn?= , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Riley Andrews , linux-media@vger.kernel.org Subject: Re: [PATCH] android: fix warning when releasing active sync point Message-ID: <20151215190008.GE883@joana> References: <20151215012955.GA28277@dtor-ws> <20151215092601.GI3189@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151215092601.GI3189@phenom.ffwll.local> Sender: linux-media-owner@vger.kernel.org List-ID: 2015-12-15 Daniel Vetter : > On Mon, Dec 14, 2015 at 05:29:55PM -0800, Dmitry Torokhov wrote: > > Userspace can close the sync device while there are still active fence > > points, in which case kernel produces the following warning: > > > > [ 43.853176] ------------[ cut here ]------------ > > [ 43.857834] WARNING: CPU: 0 PID: 892 at /mnt/host/source/src/third_party/kernel/v3.18/drivers/staging/android/sync.c:439 android_fence_release+0x88/0x104() > > [ 43.871741] CPU: 0 PID: 892 Comm: Binder_5 Tainted: G U 3.18.0-07661-g0550ce9 #1 > > [ 43.880176] Hardware name: Google Tegra210 Smaug Rev 1+ (DT) > > [ 43.885834] Call trace: > > [ 43.888294] [] dump_backtrace+0x0/0x10c > > [ 43.893697] [] show_stack+0x10/0x1c > > [ 43.898756] [] dump_stack+0x74/0xb8 > > [ 43.903814] [] warn_slowpath_common+0x84/0xb0 > > [ 43.909736] [] warn_slowpath_null+0x14/0x20 > > [ 43.915482] [] android_fence_release+0x84/0x104 > > [ 43.921582] [] fence_release+0x104/0x134 > > [ 43.927066] [] sync_fence_free+0x74/0x9c > > [ 43.932552] [] sync_fence_release+0x34/0x48 > > [ 43.938304] [] __fput+0x100/0x1b8 > > [ 43.943185] [] ____fput+0x8/0x14 > > [ 43.947982] [] task_work_run+0xb0/0xe4 > > [ 43.953297] [] do_notify_resume+0x44/0x5c > > [ 43.958867] ---[ end trace 5a2aa4027cc5d171 ]--- > > > > Let's fix it by introducing a new optional callback (disable_signaling) > > to fence operations so that drivers can do proper clean ups when we > > remove last callback for given fence. > > > > Reviewed-by: Andrew Bresticker > > Signed-off-by: Dmitry Torokhov > > --- > > drivers/dma-buf/fence.c | 6 +++++- > > drivers/staging/android/sync.c | 8 ++++++++ > > include/linux/fence.h | 2 ++ > > 3 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c > > index 7b05dbe..0ed73ad 100644 > > --- a/drivers/dma-buf/fence.c > > +++ b/drivers/dma-buf/fence.c > > @@ -304,8 +304,12 @@ fence_remove_callback(struct fence *fence, struct fence_cb *cb) > > spin_lock_irqsave(fence->lock, flags); > > > > ret = !list_empty(&cb->node); > > - if (ret) > > + if (ret) { > > list_del_init(&cb->node); > > + if (list_empty(&fence->cb_list)) > > + if (fence->ops->disable_signaling) > > + fence->ops->disable_signaling(fence); > > What exactly is the bug here? A fence with no callbacks registered any > more shouldn't have any problem. Why exactly does this blow up? The WARN_ON is probably this one: https://android.googlesource.com/kernel/common/+/android-3.18/drivers/staging/android/sync.c#433 I've been wondering in the last few days if this warning is really necessary. If the user is closing a sync_timeline that has unsignalled fences it should probably be aware of that already. Then I think it is okay to remove the the sync_pt from the active_list at the release-time. In fact I've already prepared a patch doing that. Thoughts? Gustavo