From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [RFC 1/5] dma-buf/fence: add .teardown() ops Date: Tue, 12 Jul 2016 12:51:22 +0200 Message-ID: <20160712105122.GD23520@phenom.ffwll.local> References: <1466695790-2833-1-git-send-email-gustavo@padovan.org> <1466695790-2833-2-git-send-email-gustavo@padovan.org> <20160623204814.GC1086@nuc-i3427.alporthouse.com> <20160624131900.GB2503@joana> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x22a.google.com (mail-wm0-x22a.google.com [IPv6:2a00:1450:400c:c09::22a]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5E57F6E4DE for ; Tue, 12 Jul 2016 10:51:27 +0000 (UTC) Received: by mail-wm0-x22a.google.com with SMTP id f126so123166330wma.1 for ; Tue, 12 Jul 2016 03:51:27 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20160624131900.GB2503@joana> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Gustavo Padovan , Chris Wilson , dri-devel@lists.freedesktop.org, marcheu@google.com, Daniel Stone , seanpaul@google.com, Daniel Vetter , linux-kernel@vger.kernel.org, laurent.pinchart@ideasonboard.com, Gustavo Padovan , John Harrison , m.chehab@samsung.com List-Id: dri-devel@lists.freedesktop.org T24gRnJpLCBKdW4gMjQsIDIwMTYgYXQgMTA6MTk6MDBBTSAtMDMwMCwgR3VzdGF2byBQYWRvdmFu IHdyb3RlOgo+IDIwMTYtMDYtMjMgQ2hyaXMgV2lsc29uIDxjaHJpc0BjaHJpcy13aWxzb24uY28u dWs+Ogo+IAo+ID4gT24gVGh1LCBKdW4gMjMsIDIwMTYgYXQgMTI6Mjk6NDZQTSAtMDMwMCwgR3Vz dGF2byBQYWRvdmFuIHdyb3RlOgo+ID4gPiBGcm9tOiBHdXN0YXZvIFBhZG92YW4gPGd1c3Rhdm8u cGFkb3ZhbkBjb2xsYWJvcmEuY28udWs+Cj4gPiA+IAo+ID4gPiBmZW5jZV9hcnJheSByZXF1aXJl cyBhIGZ1bmN0aW9uIHRvIGNsZWFuIHVwIGl0cyBzdGF0ZSBiZWZvcmUgd2UKPiA+ID4gYXJlIGFi bGUgdG8gY2FsbCBmZW5jZV9wdXQoKSBhbmQgcmVsZWFzZSBpdC4KPiA+IAo+ID4gQW4gZXhwbGFu YXRpb24gYWxvbmcgdGhlIGxpbmVzIG9mOgo+ID4gCj4gPiBBcyB0aGUgYXJyYXkgb2YgZmVuY2Ug Y2FsbGJhY2tzIGhlbGQgYnkgYW4gYWN0aXZlIHN0cnVjdCBmZW5jZV9hcnJheQo+ID4gZWFjaCBo YXMgYSByZWZlcmVuY2UgdG8gdGhlIHN0cnVjdCBmZW5jZV9hcnJheSwgd2hlbiB0aGUgb3duZXIg b2YgdGhlCj4gPiBmZW5jZV9hcnJheSBpcyBmcmVlZCBpdCBtdXN0IGRpc3Bvc2Ugb2YgdGhlIGNh bGxiYWNrIHJlZmVyZW5jZXMgYmVmb3JlCj4gPiBpdCBjYW4gZnJlZSB0aGUgZmVuY2VfYXJyYXku IFRoaXMgY2FuIG5vdCBoYXBwZW4gc2ltcGx5IGR1cmluZwo+ID4gZmVuY2VfcmVsZWFzZSgpIGJl Y2F1c2Ugb2YgdGhlIGV4dHJhIHJlZmVyZW5jZXMgYW5kIHNvIHdlIG5lZWQgYSBuZXcKPiA+IGZ1 bmN0aW9uIHRvIHJ1biBiZWZvcmUgdGhlIGZpbmFsIGZlbmNlX3B1dCgpLgo+ID4gCj4gPiB3b3Vs ZCBoZWxwLCBpdCBpcyBub3QgdW50aWwgeW91IHVzZSBpdCBpbiA1LzUgdGhhdCBpdCBiZWNvbWVz IGFwcGFyZW50Cj4gPiB3aHkgaXQgaXMgbmVlZGVkLgo+IAo+IFRoYXQgaXMgbXVjaCBiZXR0ZXIg ZXhwbGFuYXRpb24uIFRoYW5rcyEKCldoYXQgaGFwcGVucyBpZiB0aGUgb3duZXIgb2YgdGhlIGZl bmNlX2FycmF5IGlzbid0IHRoZSBsYXN0IHJlZmVyZW5jZQpob2xkZXIgYW55IG1vcmU/IFdoYXQg aWYgdGhlcmUncyBhIDJuZCBzeW5jX2ZpbGUgdGhhdCBub3cgc3RvcHMgd29ya2luZwpiZWNhdXNl IHRoZSBjYWxsYmFja3Mgd2VudCBwb29mPyBTb21lIG90aGVyIGRyaXZlciB0aGF0IHJlZ2lzdGVy ZWQKY2FsbGJhY2tzPwoKR2VuZXJhbGx5IG1peGluZyByZWZjb3VudGluZyB3aXRoIGV4cGxpY2l0 IHRlYXJkb3duIGlzIHJlYWxseSB0cmlja3ksCmZyYWdpbGUgYW5kIHRlbmRzIHRvIG5vdCB3b3Jr LiBUaGlzIHNtZWxscyBmaXNoeS4KCldoeSBleGFjdGx5IGRvIHdlIGhhdmUgYSByZWZlcmVuY2Ug Y291bnQgbG9vcCBoZXJlIGluIHRoZSBmaXJzdCBwbGFjZSB0aGF0CndlIG5lZWQgdG8gYnJlYWsg dXAgdXNpbmcgZmVuY2VfdGVhcmRvd24/Ci1EYW5pZWwKLS0gCkRhbmllbCBWZXR0ZXIKU29mdHdh cmUgRW5naW5lZXIsIEludGVsIENvcnBvcmF0aW9uCmh0dHA6Ly9ibG9nLmZmd2xsLmNoCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWls aW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZy ZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752778AbcGLKv2 (ORCPT ); Tue, 12 Jul 2016 06:51:28 -0400 Received: from mail-wm0-f48.google.com ([74.125.82.48]:35780 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751014AbcGLKv1 (ORCPT ); Tue, 12 Jul 2016 06:51:27 -0400 Date: Tue, 12 Jul 2016 12:51:22 +0200 From: Daniel Vetter To: Gustavo Padovan , Chris Wilson , dri-devel@lists.freedesktop.org, marcheu@google.com, Daniel Stone , seanpaul@google.com, Daniel Vetter , linux-kernel@vger.kernel.org, laurent.pinchart@ideasonboard.com, Gustavo Padovan , John Harrison , m.chehab@samsung.com Subject: Re: [RFC 1/5] dma-buf/fence: add .teardown() ops Message-ID: <20160712105122.GD23520@phenom.ffwll.local> Mail-Followup-To: Gustavo Padovan , Chris Wilson , dri-devel@lists.freedesktop.org, marcheu@google.com, Daniel Stone , seanpaul@google.com, linux-kernel@vger.kernel.org, laurent.pinchart@ideasonboard.com, Gustavo Padovan , John Harrison , m.chehab@samsung.com References: <1466695790-2833-1-git-send-email-gustavo@padovan.org> <1466695790-2833-2-git-send-email-gustavo@padovan.org> <20160623204814.GC1086@nuc-i3427.alporthouse.com> <20160624131900.GB2503@joana> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160624131900.GB2503@joana> X-Operating-System: Linux phenom 4.6.0-rc5+ User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jun 24, 2016 at 10:19:00AM -0300, Gustavo Padovan wrote: > 2016-06-23 Chris Wilson : > > > On Thu, Jun 23, 2016 at 12:29:46PM -0300, Gustavo Padovan wrote: > > > From: Gustavo Padovan > > > > > > fence_array requires a function to clean up its state before we > > > are able to call fence_put() and release it. > > > > An explanation along the lines of: > > > > As the array of fence callbacks held by an active struct fence_array > > each has a reference to the struct fence_array, when the owner of the > > fence_array is freed it must dispose of the callback references before > > it can free the fence_array. This can not happen simply during > > fence_release() because of the extra references and so we need a new > > function to run before the final fence_put(). > > > > would help, it is not until you use it in 5/5 that it becomes apparent > > why it is needed. > > That is much better explanation. Thanks! What happens if the owner of the fence_array isn't the last reference holder any more? What if there's a 2nd sync_file that now stops working because the callbacks went poof? Some other driver that registered callbacks? Generally mixing refcounting with explicit teardown is really tricky, fragile and tends to not work. This smells fishy. Why exactly do we have a reference count loop here in the first place that we need to break up using fence_teardown? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch