From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.fireflyinternet.com ([109.228.58.192]:57614 "EHLO fireflyinternet.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752209AbeEKHYF (ORCPT ); Fri, 11 May 2018 03:24:05 -0400 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT To: Ezequiel Garcia , "Gustavo Padovan" , "Sumit Semwal" From: Chris Wilson In-Reply-To: Cc: kernel@collabora.com, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org References: <20180509201449.27452-1-ezequiel@collabora.com> <1525905732.3381.6.camel@padovan.org> Message-ID: <152602343698.22269.13414355228515737873@mail.alporthouse.com> Subject: Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error Date: Fri, 11 May 2018 08:23:56 +0100 Sender: linux-media-owner@vger.kernel.org List-ID: Quoting Ezequiel Garcia (2018-05-10 13:51:56) > On Wed, 2018-05-09 at 19:42 -0300, Gustavo Padovan wrote: > > Hi Ezequiel, > > > > On Wed, 2018-05-09 at 17:14 -0300, Ezequiel Garcia wrote: > > > 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. > > > > > > Signed-off-by: Ezequiel Garcia > > > --- > > > drivers/dma-buf/dma-fence.c | 10 ++++++---- > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma- > > > fence.c > > > index 4edb9fd3cf47..298b440c5b68 100644 > > > --- a/drivers/dma-buf/dma-fence.c > > > +++ b/drivers/dma-buf/dma-fence.c > > > @@ -226,7 +226,8 @@ EXPORT_SYMBOL(dma_fence_enable_sw_signaling); > > > * > > > * Note that the callback can be called from an atomic context. If > > > * fence is already signaled, this function will return -ENOENT (and > > > - * *not* call the callback) > > > + * *not* call the callback). If the fence is error-signaled, this > > > + * function returns the fence error. > > > * > > > * Add a software callback to the fence. Same restrictions apply to > > > * refcount as it does to dma_fence_wait, however the caller doesn't > > > need to > > > @@ -235,8 +236,8 @@ EXPORT_SYMBOL(dma_fence_enable_sw_signaling); > > > * after it signals with dma_fence_signal. The callback itself can > > > be called > > > * from irq context. > > > * > > > - * Returns 0 in case of success, -ENOENT if the fence is already > > > signaled > > > - * and -EINVAL in case of error. > > > + * Returns 0 in case of success, -ENOENT (or the error value) if the > > > fence is > > > + * already signaled and -EINVAL in case of error. > > > */ > > > int dma_fence_add_callback(struct dma_fence *fence, struct > > > dma_fence_cb *cb, > > > dma_fence_func_t func) > > > @@ -250,7 +251,8 @@ int dma_fence_add_callback(struct dma_fence > > > *fence, struct dma_fence_cb *cb, > > > > > > if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags)) { > > > INIT_LIST_HEAD(&cb->node); > > > - return -ENOENT; > > > + ret = (fence->error < 0) ? fence->error : -ENOENT; > > > + return ret; > > > } > > > > It looks good to me, but I'd first go check all place we call it in the > > kernel because I have some memory of callers relying on the -ENOENT > > return code for some decision. I might be wrong though. > > > > > > I checked all users before submitting this patch. > > git grep " = dma_fence_add_callback" > drivers/gpu/drm/i915/i915_sw_fence.c: ret = dma_fence_add_callback(dma, &cb->base, func); > > And from what I could see, all of them handle the error > properly. Err, no. That error then is propagated back to userspace, and that is not part of our ABI... -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: Fri, 11 May 2018 08:23:56 +0100 Message-ID: <152602343698.22269.13414355228515737873@mail.alporthouse.com> References: <20180509201449.27452-1-ezequiel@collabora.com> <1525905732.3381.6.camel@padovan.org> 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 6A0666F048 for ; Fri, 11 May 2018 07:24:09 +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 , Gustavo Padovan , Sumit Semwal Cc: kernel@collabora.com, dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org UXVvdGluZyBFemVxdWllbCBHYXJjaWEgKDIwMTgtMDUtMTAgMTM6NTE6NTYpCj4gT24gV2VkLCAy MDE4LTA1LTA5IGF0IDE5OjQyIC0wMzAwLCBHdXN0YXZvIFBhZG92YW4gd3JvdGU6Cj4gPiBIaSBF emVxdWllbCwKPiA+IAo+ID4gT24gV2VkLCAyMDE4LTA1LTA5IGF0IDE3OjE0IC0wMzAwLCBFemVx dWllbCBHYXJjaWEgd3JvdGU6Cj4gPiA+IENoYW5nZSBob3cgZG1hX2ZlbmNlX2FkZF9jYWxsYmFj aygpIGJlaGF2ZXMsIHdoZW4gdGhlIGZlbmNlCj4gPiA+IGhhcyBlcnJvci1zaWduYWxlZCBieSB0 aGUgdGltZSBpdCBpcyBiZWluZyBhZGQuIEFmdGVyIHRoaXMgY29tbWl0LAo+ID4gPiBkbWFfZmVu Y2VfYWRkX2NhbGxiYWNrKCkgcmV0dXJucyB0aGUgZmVuY2UgZXJyb3IsIGlmIGl0Cj4gPiA+IGhh cyBlcnJvci1zaWduYWxlZCBiZWZvcmUgZG1hX2ZlbmNlX2FkZF9jYWxsYmFjaygpIGlzIGNhbGxl ZC4KPiA+ID4gCj4gPiA+IFNpZ25lZC1vZmYtYnk6IEV6ZXF1aWVsIEdhcmNpYSA8ZXplcXVpZWxA Y29sbGFib3JhLmNvbT4KPiA+ID4gLS0tCj4gPiA+ICBkcml2ZXJzL2RtYS1idWYvZG1hLWZlbmNl LmMgfCAxMCArKysrKystLS0tCj4gPiA+ICAxIGZpbGUgY2hhbmdlZCwgNiBpbnNlcnRpb25zKCsp LCA0IGRlbGV0aW9ucygtKQo+ID4gPiAKPiA+ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZG1hLWJ1 Zi9kbWEtZmVuY2UuYyBiL2RyaXZlcnMvZG1hLWJ1Zi9kbWEtCj4gPiA+IGZlbmNlLmMKPiA+ID4g aW5kZXggNGVkYjlmZDNjZjQ3Li4yOThiNDQwYzViNjggMTAwNjQ0Cj4gPiA+IC0tLSBhL2RyaXZl cnMvZG1hLWJ1Zi9kbWEtZmVuY2UuYwo+ID4gPiArKysgYi9kcml2ZXJzL2RtYS1idWYvZG1hLWZl bmNlLmMKPiA+ID4gQEAgLTIyNiw3ICsyMjYsOCBAQCBFWFBPUlRfU1lNQk9MKGRtYV9mZW5jZV9l bmFibGVfc3dfc2lnbmFsaW5nKTsKPiA+ID4gICAqCj4gPiA+ICAgKiBOb3RlIHRoYXQgdGhlIGNh bGxiYWNrIGNhbiBiZSBjYWxsZWQgZnJvbSBhbiBhdG9taWMgY29udGV4dC4gIElmCj4gPiA+ICAg KiBmZW5jZSBpcyBhbHJlYWR5IHNpZ25hbGVkLCB0aGlzIGZ1bmN0aW9uIHdpbGwgcmV0dXJuIC1F Tk9FTlQgKGFuZAo+ID4gPiAtICogKm5vdCogY2FsbCB0aGUgY2FsbGJhY2spCj4gPiA+ICsgKiAq bm90KiBjYWxsIHRoZSBjYWxsYmFjaykuIElmIHRoZSBmZW5jZSBpcyBlcnJvci1zaWduYWxlZCwg dGhpcwo+ID4gPiArICogZnVuY3Rpb24gcmV0dXJucyB0aGUgZmVuY2UgZXJyb3IuCj4gPiA+ICAg Kgo+ID4gPiAgICogQWRkIGEgc29mdHdhcmUgY2FsbGJhY2sgdG8gdGhlIGZlbmNlLiBTYW1lIHJl c3RyaWN0aW9ucyBhcHBseSB0bwo+ID4gPiAgICogcmVmY291bnQgYXMgaXQgZG9lcyB0byBkbWFf ZmVuY2Vfd2FpdCwgaG93ZXZlciB0aGUgY2FsbGVyIGRvZXNuJ3QKPiA+ID4gbmVlZCB0bwo+ID4g PiBAQCAtMjM1LDggKzIzNiw4IEBAIEVYUE9SVF9TWU1CT0woZG1hX2ZlbmNlX2VuYWJsZV9zd19z aWduYWxpbmcpOwo+ID4gPiAgICogYWZ0ZXIgaXQgc2lnbmFscyB3aXRoIGRtYV9mZW5jZV9zaWdu YWwuIFRoZSBjYWxsYmFjayBpdHNlbGYgY2FuCj4gPiA+IGJlIGNhbGxlZAo+ID4gPiAgICogZnJv bSBpcnEgY29udGV4dC4KPiA+ID4gICAqCj4gPiA+IC0gKiBSZXR1cm5zIDAgaW4gY2FzZSBvZiBz dWNjZXNzLCAtRU5PRU5UIGlmIHRoZSBmZW5jZSBpcyBhbHJlYWR5Cj4gPiA+IHNpZ25hbGVkCj4g PiA+IC0gKiBhbmQgLUVJTlZBTCBpbiBjYXNlIG9mIGVycm9yLgo+ID4gPiArICogUmV0dXJucyAw IGluIGNhc2Ugb2Ygc3VjY2VzcywgLUVOT0VOVCAob3IgdGhlIGVycm9yIHZhbHVlKSBpZiB0aGUK PiA+ID4gZmVuY2UgaXMKPiA+ID4gKyAqIGFscmVhZHkgc2lnbmFsZWQgYW5kIC1FSU5WQUwgaW4g Y2FzZSBvZiBlcnJvci4KPiA+ID4gICAqLwo+ID4gPiAgaW50IGRtYV9mZW5jZV9hZGRfY2FsbGJh Y2soc3RydWN0IGRtYV9mZW5jZSAqZmVuY2UsIHN0cnVjdAo+ID4gPiBkbWFfZmVuY2VfY2IgKmNi LAo+ID4gPiAgICAgICAgICAgICAgICAgICAgICAgIGRtYV9mZW5jZV9mdW5jX3QgZnVuYykKPiA+ ID4gQEAgLTI1MCw3ICsyNTEsOCBAQCBpbnQgZG1hX2ZlbmNlX2FkZF9jYWxsYmFjayhzdHJ1Y3Qg ZG1hX2ZlbmNlCj4gPiA+ICpmZW5jZSwgc3RydWN0IGRtYV9mZW5jZV9jYiAqY2IsCj4gPiA+ICAK PiA+ID4gICAgIGlmICh0ZXN0X2JpdChETUFfRkVOQ0VfRkxBR19TSUdOQUxFRF9CSVQsICZmZW5j ZS0+ZmxhZ3MpKSB7Cj4gPiA+ICAgICAgICAgICAgIElOSVRfTElTVF9IRUFEKCZjYi0+bm9kZSk7 Cj4gPiA+IC0gICAgICAgICAgIHJldHVybiAtRU5PRU5UOwo+ID4gPiArICAgICAgICAgICByZXQg PSAoZmVuY2UtPmVycm9yIDwgMCkgPyBmZW5jZS0+ZXJyb3IgOiAtRU5PRU5UOwo+ID4gPiArICAg ICAgICAgICByZXR1cm4gcmV0Owo+ID4gPiAgICAgfQo+ID4gCj4gPiBJdCBsb29rcyBnb29kIHRv IG1lLCBidXQgSSdkIGZpcnN0IGdvIGNoZWNrIGFsbCBwbGFjZSB3ZSBjYWxsIGl0IGluIHRoZQo+ ID4ga2VybmVsIGJlY2F1c2UgSSBoYXZlIHNvbWUgbWVtb3J5IG9mIGNhbGxlcnMgcmVseWluZyBv biB0aGUgLUVOT0VOVAo+ID4gcmV0dXJuIGNvZGUgZm9yIHNvbWUgZGVjaXNpb24uIEkgbWlnaHQg YmUgd3JvbmcgdGhvdWdoLgo+ID4gCj4gPiAKPiAKPiBJIGNoZWNrZWQgYWxsIHVzZXJzIGJlZm9y ZSBzdWJtaXR0aW5nIHRoaXMgcGF0Y2guCj4gCj4gZ2l0IGdyZXAgIiA9IGRtYV9mZW5jZV9hZGRf Y2FsbGJhY2siCj4gZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9zd19mZW5jZS5jOiAgIHJldCA9 IGRtYV9mZW5jZV9hZGRfY2FsbGJhY2soZG1hLCAmY2ItPmJhc2UsIGZ1bmMpOwo+IAo+IEFuZCBm cm9tIHdoYXQgSSBjb3VsZCBzZWUsIGFsbCBvZiB0aGVtIGhhbmRsZSB0aGUgZXJyb3IKPiBwcm9w ZXJseS4KCkVyciwgbm8uIFRoYXQgZXJyb3IgdGhlbiBpcyBwcm9wYWdhdGVkIGJhY2sgdG8gdXNl cnNwYWNlLCBhbmQgdGhhdCBpcwpub3QgcGFydCBvZiBvdXIgQUJJLi4uCi1DaHJpcwpfX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGlu ZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVl ZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK