From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.fireflyinternet.com ([109.228.58.192]:53219 "EHLO fireflyinternet.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752297AbeEOMQk (ORCPT ); Tue, 15 May 2018 08:16:40 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT To: Ezequiel Garcia , "Daniel Vetter" From: Chris Wilson In-Reply-To: Cc: "Sumit Semwal" , "Gustavo Padovan" , kernel@collabora.com, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org References: <20180509201449.27452-1-ezequiel@collabora.com> <152602366168.22269.11696001916463464983@mail.alporthouse.com> <20180514164823.GH28661@phenom.ffwll.local> Message-ID: <152638659036.18532.13662508480413451560@mail.alporthouse.com> Subject: Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error Date: Tue, 15 May 2018 13:16:30 +0100 Sender: linux-media-owner@vger.kernel.org List-ID: Quoting Ezequiel Garcia (2018-05-14 22:28:31) > 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. What I have in mind for async errors is to skip the operation and propagate the error onto the next fence. Mostly because those async errors may include fatal errors such as unable to pin the backing storage for the operation, but even "trivial" errors such as an early operation failing means that this request is then subject to garbage-in, garbage-out. However, for trivial errors I would just propagate the error status (so the caller knows something went wrong if they care, but in all likelihood no one will notice) and continue on with the glitchy operation. -Chris From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error Date: Tue, 15 May 2018 13:16:30 +0100 Message-ID: <152638659036.18532.13662508480413451560@mail.alporthouse.com> 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 fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0062F6E1FD for ; Tue, 15 May 2018 12:16:42 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Ezequiel Garcia , Daniel Vetter Cc: kernel@collabora.com, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org UXVvdGluZyBFemVxdWllbCBHYXJjaWEgKDIwMTgtMDUtMTQgMjI6Mjg6MzEpCj4gT24gTW9uLCAy MDE4LTA1LTE0IGF0IDE4OjQ4ICswMjAwLCBEYW5pZWwgVmV0dGVyIHdyb3RlOgo+ID4gT24gRnJp LCBNYXkgMTEsIDIwMTggYXQgMDg6Mjc6NDFBTSArMDEwMCwgQ2hyaXMgV2lsc29uIHdyb3RlOgo+ ID4gPiBRdW90aW5nIEV6ZXF1aWVsIEdhcmNpYSAoMjAxOC0wNS0wOSAyMToxNDo0OSkKPiA+ID4g PiBDaGFuZ2UgaG93IGRtYV9mZW5jZV9hZGRfY2FsbGJhY2soKSBiZWhhdmVzLCB3aGVuIHRoZSBm ZW5jZQo+ID4gPiA+IGhhcyBlcnJvci1zaWduYWxlZCBieSB0aGUgdGltZSBpdCBpcyBiZWluZyBh ZGQuIEFmdGVyIHRoaXMgY29tbWl0LAo+ID4gPiA+IGRtYV9mZW5jZV9hZGRfY2FsbGJhY2soKSBy ZXR1cm5zIHRoZSBmZW5jZSBlcnJvciwgaWYgaXQKPiA+ID4gPiBoYXMgZXJyb3Itc2lnbmFsZWQg YmVmb3JlIGRtYV9mZW5jZV9hZGRfY2FsbGJhY2soKSBpcyBjYWxsZWQuCj4gPiA+IAo+ID4gPiBX aHk/IFdoYXQgcHJvYmxlbSBhcmUgeW91IHRyeWluZyB0byBzb2x2ZT8gZmVuY2UtPmVycm9yIGRv ZXMgbm90IGltcGx5Cj4gPiA+IHRoYXQgdGhlIGZlbmNlIGhhcyB5ZXQgYmVlbiBzaWduYWxlZCwg YW5kIHRoZSBjYWxsZXIgd2FudHMgYSBjYWxsYmFjawo+ID4gPiB3aGVuIGl0IGlzIHNpZ25hbGVk Lgo+ID4gCj4gPiBPbiB0b3AgdGhpcyBpcyBpbmNvc2lzdGVudCwgZS5nLiB3ZSBkb24ndCBkbyB0 aGUgc2FtZSBmb3IgYW55IG9mIHRoZSBvdGhlcgo+ID4gZG1hX2ZlbmNlIGludGVyZmFjZXMuIFBs dXMgdGhlcmUncyB0aGUgaXNzdWUgdGhhdCB5b3UgbWlnaHQgYWxpYXMgZXJybm8KPiA+IHZhbHVl cyB3aXRoIGZlbmNlIGVycm5vIHZhbHVlcy4KPiA+IAo+IAo+IFJpZ2h0Lgo+IAo+ID4gSSB0aGlu ayBrZWVwaW5nIHRoZSBlcnJvciBjb2RlcyBmcm9tIHRoZSBmdW5jdGlvbnMgeW91J3JlIGNhbGxp bmcgZGlzdGluY3QKPiA+IGZyb20gdGhlIGVycm9yIGNvZGUgb2YgdGhlIGZlbmNlIGl0c2VsZiBt YWtlcyBhIGxvdCBvZiBzZW5zZS4gVGhlIGZpcnN0Cj4gPiB0ZWxscyB5b3Ugd2hldGhlciB5b3Vy IHJlcXVlc3Qgd29ya2VkIG91dCAob3Igd2h5IG5vdCksIHRoZSBzZWNvbmQgdGVsbHMKPiA+IHlv dSB3aGV0aGVyIHRoZSBhc3luY2hyb25vdXMgZG1hIG9wZXJhdGlvbiAoZ3B1IHJlbmRlcmluZywg cGFnZSBmbGlwLAo+ID4gd2hhdGV2ZXIpIHRoYXQgdGhlIGRtYV9mZW5jZSByZXByZXNlbnRzIHdv cmtlZCBvdXQgKG9yIHdoeSBub3QpLiBUaGF0J3MgMgo+ID4gZGlzdGluY3QgdGhpbmdzIGltby4K PiA+IAo+ID4gTWlnaHQgYmUgZ29vZCB0byBzaG93IHVzIHRoZSBkcml2ZXIgY29kZSB0aGF0IG5l ZWRzIHRoaXMgYmVoYXZpb3VyIHNvIHdlCj4gPiBjYW4gZGlzY3VzcyBob3cgdG8gYmVzdCBoYW5k bGUgeW91ciB1c2UtY2FzZS4KPiA+IAo+IAo+IFRoaXMgY2hhbmdlIGFyb3NlIHdoaWxlIGRpc2N1 c3NpbmcgdGhlIGluLWZlbmNlcyBzdXBwb3J0IGZvciB2aWRlbzRsaW51eC4KPiBIZXJlJ3MgdGhl IHBhdGNoIHRoYXQgY2FsbHMgZG1hX2ZlbmNlX2FkZF9jYWxsYmFjayBodHRwczovL2xrbWwub3Jn L2xrbWwvMjAxOC81LzQvNzY2Lgo+IAo+IFRoZSBjb2RlIHNuaXBwZXQgY3VycmVudGx5IGxvb2tz IHNvbWV0aGluZyBsaWtlIHRoaXM6Cj4gCj4gICAgICAgICBpZiAodmItPmluX2ZlbmNlKSB7Cj4g ICAgICAgICAgICAgICAgIHJldCA9IGRtYV9mZW5jZV9hZGRfY2FsbGJhY2sodmItPmluX2ZlbmNl LCAmdmItPmZlbmNlX2NiLAo+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgCj4gICAg ICAgICAgICAgIHZiMl9xYnVmX2ZlbmNlX2NiKTsKPiAgICAgICAgICAgICAgICAgLyogaXMgdGhl IGZlbmNlIHNpZ25hbGVkPyAqLwo+ICAgICAgICAgICAgICAgICBpZiAocmV0ID09IC1FTk9FTlQp IHsKPiAgICAgICAgIAo+ICAgICAgICAgICAgICAgICBkbWFfZmVuY2VfcHV0KHZiLT5pbl9mZW5j ZSk7Cj4gICAgICAgICAgICAgICAgICAgICAgICAgdmItPmluX2ZlbmNlID0gTlVMTDsKPiAgICAg ICAgICAgICAgICAgfSBlbHNlIGlmIChyZXQpCj4gewo+ICAgICAgICAgICAgICAgICAgICAgICAg IGdvdG8gdW5sb2NrOwo+ICAgICAgICAgICAgICAgICB9Cj4gICAgICAgICB9Cj4gCj4gSW4gdGhp cyB1c2UgY2FzZSwgaWYgdGhlIGNhbGxiYWNrIGlzIGFkZGVkIHN1Y2Nlc3NmdWxseSwKPiB0aGUg dmlkZW80bGludXggY29yZSBkZWZlcnMgdGhlIGFjdGl2YXRpb24gb2YgdGhlIGJ1ZmZlcgo+IHVu dGlsIHRoZSBmZW5jZSBzaWduYWxzLgo+IAo+IElmIHRoZSBmZW5jZSBpcyBzaWduYWxlZCAoY3Vy cmVudGx5IGRpc3JlZ2FyZGluZyBvZiBlcnJvcnMpCj4gdGhlbiB0aGUgYnVmZmVyIGlzIGFzc3Vt ZWQgdG8gYmUgcmVhZHkgdG8gYmUgYWN0aXZhdGVkLAo+IGFuZCBzbyBpdCBnZXRzIHF1ZXVlZCBm b3IgaGFyZHdhcmUgdXNhZ2UuCj4gCj4gR2l2aW5nIHNvbWUgbW9yZSB0aG91Z2h0IHRvIHRoaXMs IEknbSBub3Qgc28gc3VyZSB3aGF0IGlzCj4gdGhlIHJpZ2h0IGFjdGlvbiBpZiBhIGZlbmNlIHNp Z25hbGVkIHdpdGggZXJyb3IuIEluIHRoaXMgY2FzZSwKPiBpdCBhcHBlYXJzIHRvIG1lIHRoYXQg d2Ugc2hvdWxkbid0IGJlIHVzaW5nIHRoaXMgYnVmZmVyCj4gaWYgaXRzIGluLWZlbmNlIGlzIGlu IGVycm9yLCBidXQgcGVyaGFwcyBJJ20gbWlzc2luZwo+IHNvbWV0aGluZy4KCldoYXQgSSBoYXZl IGluIG1pbmQgZm9yIGFzeW5jIGVycm9ycyBpcyB0byBza2lwIHRoZSBvcGVyYXRpb24gYW5kCnBy b3BhZ2F0ZSB0aGUgZXJyb3Igb250byB0aGUgbmV4dCBmZW5jZS4gTW9zdGx5IGJlY2F1c2UgdGhv c2UgYXN5bmMKZXJyb3JzIG1heSBpbmNsdWRlIGZhdGFsIGVycm9ycyBzdWNoIGFzIHVuYWJsZSB0 byBwaW4gdGhlIGJhY2tpbmcKc3RvcmFnZSBmb3IgdGhlIG9wZXJhdGlvbiwgYnV0IGV2ZW4gInRy aXZpYWwiIGVycm9ycyBzdWNoIGFzIGFuIGVhcmx5Cm9wZXJhdGlvbiBmYWlsaW5nIG1lYW5zIHRo YXQgdGhpcyByZXF1ZXN0IGlzIHRoZW4gc3ViamVjdCB0byBnYXJiYWdlLWluLApnYXJiYWdlLW91 dC4gSG93ZXZlciwgZm9yIHRyaXZpYWwgZXJyb3JzIEkgd291bGQganVzdCBwcm9wYWdhdGUgdGhl CmVycm9yIHN0YXR1cyAoc28gdGhlIGNhbGxlciBrbm93cyBzb21ldGhpbmcgd2VudCB3cm9uZyBp ZiB0aGV5IGNhcmUsIGJ1dAppbiBhbGwgbGlrZWxpaG9vZCBubyBvbmUgd2lsbCBub3RpY2UpIGFu ZCBjb250aW51ZSBvbiB3aXRoIHRoZSBnbGl0Y2h5Cm9wZXJhdGlvbi4KLUNocmlzCl9fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5n IGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVk ZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo=