From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from bhuna.collabora.co.uk ([46.235.227.227]:34638 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752708AbeENV36 (ORCPT ); Mon, 14 May 2018 17:29:58 -0400 Message-ID: Subject: Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error From: Ezequiel Garcia To: Daniel Vetter , Chris Wilson Cc: Sumit Semwal , Gustavo Padovan , kernel@collabora.com, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org Date: Mon, 14 May 2018 18:28:31 -0300 In-Reply-To: <20180514164823.GH28661@phenom.ffwll.local> References: <20180509201449.27452-1-ezequiel@collabora.com> <152602366168.22269.11696001916463464983@mail.alporthouse.com> <20180514164823.GH28661@phenom.ffwll.local> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-media-owner@vger.kernel.org List-ID: On Mon, 2018-05-14 at 18:48 +0200, Daniel Vetter wrote: > On Fri, May 11, 2018 at 08:27:41AM +0100, Chris Wilson wrote: > > Quoting Ezequiel Garcia (2018-05-09 21:14:49) > > > Change how dma_fence_add_callback() behaves, when the fence > > > has error-signaled by the time it is being add. After this commit, > > > dma_fence_add_callback() returns the fence error, if it > > > has error-signaled before dma_fence_add_callback() is called. > > > > Why? What problem are you trying to solve? fence->error does not imply > > that the fence has yet been signaled, and the caller wants a callback > > when it is signaled. > > On top this is incosistent, e.g. we don't do the same for any of the other > dma_fence interfaces. Plus there's the issue that you might alias errno > values with fence errno values. > Right. > I think keeping the error codes from the functions you're calling distinct > from the error code of the fence itself makes a lot of sense. The first > tells you whether your request worked out (or why not), the second tells > you whether the asynchronous dma operation (gpu rendering, page flip, > whatever) that the dma_fence represents worked out (or why not). That's 2 > distinct things imo. > > Might be good to show us the driver code that needs this behaviour so we > can discuss how to best handle your use-case. > This change arose while discussing the in-fences support for video4linux. Here's the patch that calls dma_fence_add_callback https://lkml.org/lkml/2018/5/4/766. The code snippet currently looks something like this: if (vb->in_fence) { ret = dma_fence_add_callback(vb->in_fence, &vb->fence_cb, vb2_qbuf_fence_cb); /* is the fence signaled? */ if (ret == -ENOENT) { dma_fence_put(vb->in_fence); vb->in_fence = NULL; } else if (ret) { goto unlock; } } In this use case, if the callback is added successfully, the video4linux core defers the activation of the buffer until the fence signals. If the fence is signaled (currently disregarding of errors) then the buffer is assumed to be ready to be activated, and so it gets queued for hardware usage. Giving some more thought to this, I'm not so sure what is the right action if a fence signaled with error. In this case, it appears to me that we shouldn't be using this buffer if its in-fence is in error, but perhaps I'm missing something. Thanks! Eze From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ezequiel Garcia Subject: Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error Date: Mon, 14 May 2018 18:28:31 -0300 Message-ID: References: <20180509201449.27452-1-ezequiel@collabora.com> <152602366168.22269.11696001916463464983@mail.alporthouse.com> <20180514164823.GH28661@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2FFEA89D2F for ; Mon, 14 May 2018 21:29:59 +0000 (UTC) In-Reply-To: <20180514164823.GH28661@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 , Chris Wilson Cc: kernel@collabora.com, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org T24gTW9uLCAyMDE4LTA1LTE0IGF0IDE4OjQ4ICswMjAwLCBEYW5pZWwgVmV0dGVyIHdyb3RlOgo+ IE9uIEZyaSwgTWF5IDExLCAyMDE4IGF0IDA4OjI3OjQxQU0gKzAxMDAsIENocmlzIFdpbHNvbiB3 cm90ZToKPiA+IFF1b3RpbmcgRXplcXVpZWwgR2FyY2lhICgyMDE4LTA1LTA5IDIxOjE0OjQ5KQo+ ID4gPiBDaGFuZ2UgaG93IGRtYV9mZW5jZV9hZGRfY2FsbGJhY2soKSBiZWhhdmVzLCB3aGVuIHRo ZSBmZW5jZQo+ID4gPiBoYXMgZXJyb3Itc2lnbmFsZWQgYnkgdGhlIHRpbWUgaXQgaXMgYmVpbmcg YWRkLiBBZnRlciB0aGlzIGNvbW1pdCwKPiA+ID4gZG1hX2ZlbmNlX2FkZF9jYWxsYmFjaygpIHJl dHVybnMgdGhlIGZlbmNlIGVycm9yLCBpZiBpdAo+ID4gPiBoYXMgZXJyb3Itc2lnbmFsZWQgYmVm b3JlIGRtYV9mZW5jZV9hZGRfY2FsbGJhY2soKSBpcyBjYWxsZWQuCj4gPiAKPiA+IFdoeT8gV2hh dCBwcm9ibGVtIGFyZSB5b3UgdHJ5aW5nIHRvIHNvbHZlPyBmZW5jZS0+ZXJyb3IgZG9lcyBub3Qg aW1wbHkKPiA+IHRoYXQgdGhlIGZlbmNlIGhhcyB5ZXQgYmVlbiBzaWduYWxlZCwgYW5kIHRoZSBj YWxsZXIgd2FudHMgYSBjYWxsYmFjawo+ID4gd2hlbiBpdCBpcyBzaWduYWxlZC4KPiAKPiBPbiB0 b3AgdGhpcyBpcyBpbmNvc2lzdGVudCwgZS5nLiB3ZSBkb24ndCBkbyB0aGUgc2FtZSBmb3IgYW55 IG9mIHRoZSBvdGhlcgo+IGRtYV9mZW5jZSBpbnRlcmZhY2VzLiBQbHVzIHRoZXJlJ3MgdGhlIGlz c3VlIHRoYXQgeW91IG1pZ2h0IGFsaWFzIGVycm5vCj4gdmFsdWVzIHdpdGggZmVuY2UgZXJybm8g dmFsdWVzLgo+IAoKUmlnaHQuCgo+IEkgdGhpbmsga2VlcGluZyB0aGUgZXJyb3IgY29kZXMgZnJv bSB0aGUgZnVuY3Rpb25zIHlvdSdyZSBjYWxsaW5nIGRpc3RpbmN0Cj4gZnJvbSB0aGUgZXJyb3Ig Y29kZSBvZiB0aGUgZmVuY2UgaXRzZWxmIG1ha2VzIGEgbG90IG9mIHNlbnNlLiBUaGUgZmlyc3QK PiB0ZWxscyB5b3Ugd2hldGhlciB5b3VyIHJlcXVlc3Qgd29ya2VkIG91dCAob3Igd2h5IG5vdCks IHRoZSBzZWNvbmQgdGVsbHMKPiB5b3Ugd2hldGhlciB0aGUgYXN5bmNocm9ub3VzIGRtYSBvcGVy YXRpb24gKGdwdSByZW5kZXJpbmcsIHBhZ2UgZmxpcCwKPiB3aGF0ZXZlcikgdGhhdCB0aGUgZG1h X2ZlbmNlIHJlcHJlc2VudHMgd29ya2VkIG91dCAob3Igd2h5IG5vdCkuIFRoYXQncyAyCj4gZGlz dGluY3QgdGhpbmdzIGltby4KPiAKPiBNaWdodCBiZSBnb29kIHRvIHNob3cgdXMgdGhlIGRyaXZl ciBjb2RlIHRoYXQgbmVlZHMgdGhpcyBiZWhhdmlvdXIgc28gd2UKPiBjYW4gZGlzY3VzcyBob3cg dG8gYmVzdCBoYW5kbGUgeW91ciB1c2UtY2FzZS4KPiAKClRoaXMgY2hhbmdlIGFyb3NlIHdoaWxl IGRpc2N1c3NpbmcgdGhlIGluLWZlbmNlcyBzdXBwb3J0IGZvciB2aWRlbzRsaW51eC4KSGVyZSdz IHRoZSBwYXRjaCB0aGF0IGNhbGxzIGRtYV9mZW5jZV9hZGRfY2FsbGJhY2sgaHR0cHM6Ly9sa21s Lm9yZy9sa21sLzIwMTgvNS80Lzc2Ni4KClRoZSBjb2RlIHNuaXBwZXQgY3VycmVudGx5IGxvb2tz IHNvbWV0aGluZyBsaWtlIHRoaXM6CgoJaWYgKHZiLT5pbl9mZW5jZSkgewoJCXJldCA9IGRtYV9m ZW5jZV9hZGRfY2FsbGJhY2sodmItPmluX2ZlbmNlLCAmdmItPmZlbmNlX2NiLAoJCQkJCgkgICAg IHZiMl9xYnVmX2ZlbmNlX2NiKTsKCQkvKiBpcyB0aGUgZmVuY2Ugc2lnbmFsZWQ/ICovCgkJaWYg KHJldCA9PSAtRU5PRU5UKSB7CgkKCQlkbWFfZmVuY2VfcHV0KHZiLT5pbl9mZW5jZSk7CgkJCXZi LT5pbl9mZW5jZSA9IE5VTEw7CgkJfSBlbHNlIGlmIChyZXQpCnsKCQkJZ290byB1bmxvY2s7CgkJ fQoJfQoKSW4gdGhpcyB1c2UgY2FzZSwgaWYgdGhlIGNhbGxiYWNrIGlzIGFkZGVkIHN1Y2Nlc3Nm dWxseSwKdGhlIHZpZGVvNGxpbnV4IGNvcmUgZGVmZXJzIHRoZSBhY3RpdmF0aW9uIG9mIHRoZSBi dWZmZXIKdW50aWwgdGhlIGZlbmNlIHNpZ25hbHMuCgpJZiB0aGUgZmVuY2UgaXMgc2lnbmFsZWQg KGN1cnJlbnRseSBkaXNyZWdhcmRpbmcgb2YgZXJyb3JzKQp0aGVuIHRoZSBidWZmZXIgaXMgYXNz dW1lZCB0byBiZSByZWFkeSB0byBiZSBhY3RpdmF0ZWQsCmFuZCBzbyBpdCBnZXRzIHF1ZXVlZCBm b3IgaGFyZHdhcmUgdXNhZ2UuCgpHaXZpbmcgc29tZSBtb3JlIHRob3VnaHQgdG8gdGhpcywgSSdt IG5vdCBzbyBzdXJlIHdoYXQgaXMKdGhlIHJpZ2h0IGFjdGlvbiBpZiBhIGZlbmNlIHNpZ25hbGVk IHdpdGggZXJyb3IuIEluIHRoaXMgY2FzZSwKaXQgYXBwZWFycyB0byBtZSB0aGF0IHdlIHNob3Vs ZG4ndCBiZSB1c2luZyB0aGlzIGJ1ZmZlcgppZiBpdHMgaW4tZmVuY2UgaXMgaW4gZXJyb3IsIGJ1 dCBwZXJoYXBzIEknbSBtaXNzaW5nCnNvbWV0aGluZy4KClRoYW5rcyEKRXplCgpfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBs aXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVz a3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK