From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from bhuna.collabora.co.uk ([46.235.227.227]:34018 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935589AbeEJMxU (ORCPT ); Thu, 10 May 2018 08:53:20 -0400 Message-ID: Subject: Re: [PATCH] dma-fence: Make dma_fence_add_callback() fail if signaled with error From: Ezequiel Garcia To: Gustavo Padovan , Sumit Semwal Cc: linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, kernel@collabora.com Date: Thu, 10 May 2018 09:51:56 -0300 In-Reply-To: <1525905732.3381.6.camel@padovan.org> References: <20180509201449.27452-1-ezequiel@collabora.com> <1525905732.3381.6.camel@padovan.org> 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 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); drivers/gpu/drm/scheduler/gpu_scheduler.c: r = dma_fence_add_callback(fence, &s_fence->cb, drivers/gpu/drm/scheduler/gpu_scheduler.c: r = dma_fence_add_callback(fence, &s_fence->cb, And from what I could see, all of them handle the error properly. 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: Thu, 10 May 2018 09:51:56 -0300 Message-ID: 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 bhuna.collabora.co.uk (bhuna.collabora.co.uk [46.235.227.227]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3C2B56EF2D for ; Thu, 10 May 2018 12:53:20 +0000 (UTC) In-Reply-To: <1525905732.3381.6.camel@padovan.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: 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 T24gV2VkLCAyMDE4LTA1LTA5IGF0IDE5OjQyIC0wMzAwLCBHdXN0YXZvIFBhZG92YW4gd3JvdGU6 Cj4gSGkgRXplcXVpZWwsCj4gCj4gT24gV2VkLCAyMDE4LTA1LTA5IGF0IDE3OjE0IC0wMzAwLCBF emVxdWllbCBHYXJjaWEgd3JvdGU6Cj4gPiBDaGFuZ2UgaG93IGRtYV9mZW5jZV9hZGRfY2FsbGJh Y2soKSBiZWhhdmVzLCB3aGVuIHRoZSBmZW5jZQo+ID4gaGFzIGVycm9yLXNpZ25hbGVkIGJ5IHRo ZSB0aW1lIGl0IGlzIGJlaW5nIGFkZC4gQWZ0ZXIgdGhpcyBjb21taXQsCj4gPiBkbWFfZmVuY2Vf YWRkX2NhbGxiYWNrKCkgcmV0dXJucyB0aGUgZmVuY2UgZXJyb3IsIGlmIGl0Cj4gPiBoYXMgZXJy b3Itc2lnbmFsZWQgYmVmb3JlIGRtYV9mZW5jZV9hZGRfY2FsbGJhY2soKSBpcyBjYWxsZWQuCj4g PiAKPiA+IFNpZ25lZC1vZmYtYnk6IEV6ZXF1aWVsIEdhcmNpYSA8ZXplcXVpZWxAY29sbGFib3Jh LmNvbT4KPiA+IC0tLQo+ID4gIGRyaXZlcnMvZG1hLWJ1Zi9kbWEtZmVuY2UuYyB8IDEwICsrKysr Ky0tLS0KPiA+ICAxIGZpbGUgY2hhbmdlZCwgNiBpbnNlcnRpb25zKCspLCA0IGRlbGV0aW9ucygt KQo+ID4gCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9kbWEtYnVmL2RtYS1mZW5jZS5jIGIvZHJp dmVycy9kbWEtYnVmL2RtYS0KPiA+IGZlbmNlLmMKPiA+IGluZGV4IDRlZGI5ZmQzY2Y0Ny4uMjk4 YjQ0MGM1YjY4IDEwMDY0NAo+ID4gLS0tIGEvZHJpdmVycy9kbWEtYnVmL2RtYS1mZW5jZS5jCj4g PiArKysgYi9kcml2ZXJzL2RtYS1idWYvZG1hLWZlbmNlLmMKPiA+IEBAIC0yMjYsNyArMjI2LDgg QEAgRVhQT1JUX1NZTUJPTChkbWFfZmVuY2VfZW5hYmxlX3N3X3NpZ25hbGluZyk7Cj4gPiAgICoK PiA+ICAgKiBOb3RlIHRoYXQgdGhlIGNhbGxiYWNrIGNhbiBiZSBjYWxsZWQgZnJvbSBhbiBhdG9t aWMgY29udGV4dC4gIElmCj4gPiAgICogZmVuY2UgaXMgYWxyZWFkeSBzaWduYWxlZCwgdGhpcyBm dW5jdGlvbiB3aWxsIHJldHVybiAtRU5PRU5UIChhbmQKPiA+IC0gKiAqbm90KiBjYWxsIHRoZSBj YWxsYmFjaykKPiA+ICsgKiAqbm90KiBjYWxsIHRoZSBjYWxsYmFjaykuIElmIHRoZSBmZW5jZSBp cyBlcnJvci1zaWduYWxlZCwgdGhpcwo+ID4gKyAqIGZ1bmN0aW9uIHJldHVybnMgdGhlIGZlbmNl IGVycm9yLgo+ID4gICAqCj4gPiAgICogQWRkIGEgc29mdHdhcmUgY2FsbGJhY2sgdG8gdGhlIGZl bmNlLiBTYW1lIHJlc3RyaWN0aW9ucyBhcHBseSB0bwo+ID4gICAqIHJlZmNvdW50IGFzIGl0IGRv ZXMgdG8gZG1hX2ZlbmNlX3dhaXQsIGhvd2V2ZXIgdGhlIGNhbGxlciBkb2Vzbid0Cj4gPiBuZWVk IHRvCj4gPiBAQCAtMjM1LDggKzIzNiw4IEBAIEVYUE9SVF9TWU1CT0woZG1hX2ZlbmNlX2VuYWJs ZV9zd19zaWduYWxpbmcpOwo+ID4gICAqIGFmdGVyIGl0IHNpZ25hbHMgd2l0aCBkbWFfZmVuY2Vf c2lnbmFsLiBUaGUgY2FsbGJhY2sgaXRzZWxmIGNhbgo+ID4gYmUgY2FsbGVkCj4gPiAgICogZnJv bSBpcnEgY29udGV4dC4KPiA+ICAgKgo+ID4gLSAqIFJldHVybnMgMCBpbiBjYXNlIG9mIHN1Y2Nl c3MsIC1FTk9FTlQgaWYgdGhlIGZlbmNlIGlzIGFscmVhZHkKPiA+IHNpZ25hbGVkCj4gPiAtICog YW5kIC1FSU5WQUwgaW4gY2FzZSBvZiBlcnJvci4KPiA+ICsgKiBSZXR1cm5zIDAgaW4gY2FzZSBv ZiBzdWNjZXNzLCAtRU5PRU5UIChvciB0aGUgZXJyb3IgdmFsdWUpIGlmIHRoZQo+ID4gZmVuY2Ug aXMKPiA+ICsgKiBhbHJlYWR5IHNpZ25hbGVkIGFuZCAtRUlOVkFMIGluIGNhc2Ugb2YgZXJyb3Iu Cj4gPiAgICovCj4gPiAgaW50IGRtYV9mZW5jZV9hZGRfY2FsbGJhY2soc3RydWN0IGRtYV9mZW5j ZSAqZmVuY2UsIHN0cnVjdAo+ID4gZG1hX2ZlbmNlX2NiICpjYiwKPiA+ICAJCQkgICBkbWFfZmVu Y2VfZnVuY190IGZ1bmMpCj4gPiBAQCAtMjUwLDcgKzI1MSw4IEBAIGludCBkbWFfZmVuY2VfYWRk X2NhbGxiYWNrKHN0cnVjdCBkbWFfZmVuY2UKPiA+ICpmZW5jZSwgc3RydWN0IGRtYV9mZW5jZV9j YiAqY2IsCj4gPiAgCj4gPiAgCWlmICh0ZXN0X2JpdChETUFfRkVOQ0VfRkxBR19TSUdOQUxFRF9C SVQsICZmZW5jZS0+ZmxhZ3MpKSB7Cj4gPiAgCQlJTklUX0xJU1RfSEVBRCgmY2ItPm5vZGUpOwo+ ID4gLQkJcmV0dXJuIC1FTk9FTlQ7Cj4gPiArCQlyZXQgPSAoZmVuY2UtPmVycm9yIDwgMCkgPyBm ZW5jZS0+ZXJyb3IgOiAtRU5PRU5UOwo+ID4gKwkJcmV0dXJuIHJldDsKPiA+ICAJfQo+IAo+IEl0 IGxvb2tzIGdvb2QgdG8gbWUsIGJ1dCBJJ2QgZmlyc3QgZ28gY2hlY2sgYWxsIHBsYWNlIHdlIGNh bGwgaXQgaW4gdGhlCj4ga2VybmVsIGJlY2F1c2UgSSBoYXZlIHNvbWUgbWVtb3J5IG9mIGNhbGxl cnMgcmVseWluZyBvbiB0aGUgLUVOT0VOVAo+IHJldHVybiBjb2RlIGZvciBzb21lIGRlY2lzaW9u LiBJIG1pZ2h0IGJlIHdyb25nIHRob3VnaC4KPiAKPiAKCkkgY2hlY2tlZCBhbGwgdXNlcnMgYmVm b3JlIHN1Ym1pdHRpbmcgdGhpcyBwYXRjaC4KCmdpdCBncmVwICIgPSBkbWFfZmVuY2VfYWRkX2Nh bGxiYWNrIgpkcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X3N3X2ZlbmNlLmM6ICAgcmV0ID0gZG1h X2ZlbmNlX2FkZF9jYWxsYmFjayhkbWEsICZjYi0+YmFzZSwgZnVuYyk7CmRyaXZlcnMvZ3B1L2Ry bS9zY2hlZHVsZXIvZ3B1X3NjaGVkdWxlci5jOiAgICAgICAgICAgICAgICAgICAgICByID0gZG1h X2ZlbmNlX2FkZF9jYWxsYmFjayhmZW5jZSwgJnNfZmVuY2UtPmNiLApkcml2ZXJzL2dwdS9kcm0v c2NoZWR1bGVyL2dwdV9zY2hlZHVsZXIuYzogICAgICAgICAgICAgICAgICAgICAgciA9IGRtYV9m ZW5jZV9hZGRfY2FsbGJhY2soZmVuY2UsICZzX2ZlbmNlLT5jYiwKCkFuZCBmcm9tIHdoYXQgSSBj b3VsZCBzZWUsIGFsbCBvZiB0aGVtIGhhbmRsZSB0aGUgZXJyb3IKcHJvcGVybHkuCgpUaGFua3Ms CkV6ZQpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmkt ZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6 Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK