From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gustavo Padovan Subject: Re: [PATCH v2] dma-buf/fence: Take refcount on the module that owns the fence Date: Fri, 22 Jun 2018 07:04:16 -0300 Message-ID: <1529661856.7034.404.camel@padovan.org> References: <1529660407-6266-1-git-send-email-akhilpo@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1529660407-6266-1-git-send-email-akhilpo@codeaurora.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Akhil P Oommen , sumit.semwal@linaro.org, jcrouse@codeaurora.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: linux-arm-msm@vger.kernel.org, smasetty@codeaurora.org List-Id: linux-arm-msm@vger.kernel.org SGkgQWtoaWwsCgpPbiBGcmksIDIwMTgtMDYtMjIgYXQgMTU6MTAgKzA1MzAsIEFraGlsIFAgT29t bWVuIHdyb3RlOgo+IEVhY2ggZmVuY2Ugb2JqZWN0IGhvbGRzIGZ1bmN0aW9uIHBvaW50ZXJzIG9m IHRoZSBtb2R1bGUgdGhhdAo+IGluaXRpYWxpemVkCj4gaXQuIEFsbG93aW5nIHRoZSBtb2R1bGUg dG8gdW5sb2FkIGJlZm9yZSB0aGlzIGZlbmNlJ3MgcmVsZWFzZSBpcwo+IGNhdGFzdHJvcGhpYy4g U28sIGtlZXAgYSByZWZjb3VudCBvbiB0aGUgbW9kdWxlIHVudGlsIHRoZSBmZW5jZSBpcwo+IHJl bGVhc2VkLgo+IAo+IFNpZ25lZC1vZmYtYnk6IEFraGlsIFAgT29tbWVuIDxha2hpbHBvQGNvZGVh dXJvcmEub3JnPgo+IC0tLQo+IENoYW5nZXMgaW4gdjI6Cj4gLSBhZGRlZCBkZXNjcmlwdGlvbiBm b3IgdGhlIG5ldyBmdW5jdGlvbiBwYXJhbWV0ZXIuCj4gCj4gIGRyaXZlcnMvZG1hLWJ1Zi9kbWEt ZmVuY2UuYyB8IDE2ICsrKysrKysrKysrKystLS0KPiAgaW5jbHVkZS9saW51eC9kbWEtZmVuY2Uu aCAgIHwgMTAgKysrKysrKystLQo+ICAyIGZpbGVzIGNoYW5nZWQsIDIxIGluc2VydGlvbnMoKyks IDUgZGVsZXRpb25zKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZG1hLWJ1Zi9kbWEtZmVu Y2UuYyBiL2RyaXZlcnMvZG1hLWJ1Zi9kbWEtCj4gZmVuY2UuYwo+IGluZGV4IDRlZGI5ZmQuLjJh YWE0NGUgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9kbWEtYnVmL2RtYS1mZW5jZS5jCj4gKysrIGIv ZHJpdmVycy9kbWEtYnVmL2RtYS1mZW5jZS5jCj4gQEAgLTE4LDYgKzE4LDcgQEAKPiAgICogbW9y ZSBkZXRhaWxzLgo+ICAgKi8KPiAgCj4gKyNpbmNsdWRlIDxsaW51eC9tb2R1bGUuaD4KPiAgI2lu Y2x1ZGUgPGxpbnV4L3NsYWIuaD4KPiAgI2luY2x1ZGUgPGxpbnV4L2V4cG9ydC5oPgo+ICAjaW5j bHVkZSA8bGludXgvYXRvbWljLmg+Cj4gQEAgLTE2OCw2ICsxNjksNyBAQCB2b2lkIGRtYV9mZW5j ZV9yZWxlYXNlKHN0cnVjdCBrcmVmICprcmVmKQo+ICB7Cj4gIAlzdHJ1Y3QgZG1hX2ZlbmNlICpm ZW5jZSA9Cj4gIAkJY29udGFpbmVyX29mKGtyZWYsIHN0cnVjdCBkbWFfZmVuY2UsIHJlZmNvdW50 KTsKPiArCXN0cnVjdCBtb2R1bGUgKm1vZHVsZSA9IGZlbmNlLT5vd25lcjsKPiAgCj4gIAl0cmFj ZV9kbWFfZmVuY2VfZGVzdHJveShmZW5jZSk7Cj4gIAo+IEBAIC0xNzgsNiArMTgwLDggQEAgdm9p ZCBkbWFfZmVuY2VfcmVsZWFzZShzdHJ1Y3Qga3JlZiAqa3JlZikKPiAgCQlmZW5jZS0+b3BzLT5y ZWxlYXNlKGZlbmNlKTsKPiAgCWVsc2UKPiAgCQlkbWFfZmVuY2VfZnJlZShmZW5jZSk7Cj4gKwo+ ICsJbW9kdWxlX3B1dChtb2R1bGUpOwo+ICB9Cj4gIEVYUE9SVF9TWU1CT0woZG1hX2ZlbmNlX3Jl bGVhc2UpOwo+ICAKPiBAQCAtNTQxLDYgKzU0NSw3IEBAIHN0cnVjdCBkZWZhdWx0X3dhaXRfY2Ig ewo+ICAKPiAgLyoqCj4gICAqIGRtYV9mZW5jZV9pbml0IC0gSW5pdGlhbGl6ZSBhIGN1c3RvbSBm ZW5jZS4KPiArICogQG1vZHVsZToJW2luXQl0aGUgbW9kdWxlIHRoYXQgY2FsbHMgdGhpcyBBUEkK PiAgICogQGZlbmNlOglbaW5dCXRoZSBmZW5jZSB0byBpbml0aWFsaXplCj4gICAqIEBvcHM6CVtp bl0JdGhlIGRtYV9mZW5jZV9vcHMgZm9yIG9wZXJhdGlvbnMgb24gdGhpcwo+IGZlbmNlCj4gICAq IEBsb2NrOglbaW5dCXRoZSBpcnFzYWZlIHNwaW5sb2NrIHRvIHVzZSBmb3IgbG9ja2luZwo+IHRo aXMgZmVuY2UKPiBAQCAtNTU2LDggKzU2MSw5IEBAIHN0cnVjdCBkZWZhdWx0X3dhaXRfY2Igewo+ ICAgKiB0byBjaGVjayB3aGljaCBmZW5jZSBpcyBsYXRlciBieSBzaW1wbHkgdXNpbmcgZG1hX2Zl bmNlX2xhdGVyLgo+ICAgKi8KPiAgdm9pZAo+IC1kbWFfZmVuY2VfaW5pdChzdHJ1Y3QgZG1hX2Zl bmNlICpmZW5jZSwgY29uc3Qgc3RydWN0IGRtYV9mZW5jZV9vcHMKPiAqb3BzLAo+IC0JICAgICAg IHNwaW5sb2NrX3QgKmxvY2ssIHU2NCBjb250ZXh0LCB1bnNpZ25lZCBzZXFubykKPiArX2RtYV9m ZW5jZV9pbml0KHN0cnVjdCBtb2R1bGUgKm1vZHVsZSwgc3RydWN0IGRtYV9mZW5jZSAqZmVuY2Us Cj4gKwkJY29uc3Qgc3RydWN0IGRtYV9mZW5jZV9vcHMgKm9wcywgc3BpbmxvY2tfdCAqbG9jaywK PiArCQl1NjQgY29udGV4dCwgdW5zaWduZWQgc2Vxbm8pCj4gIHsKPiAgCUJVR19PTighbG9jayk7 Cj4gIAlCVUdfT04oIW9wcyB8fCAhb3BzLT53YWl0IHx8ICFvcHMtPmVuYWJsZV9zaWduYWxpbmcg fHwKPiBAQCAtNTcxLDcgKzU3NywxMSBAQCBzdHJ1Y3QgZGVmYXVsdF93YWl0X2NiIHsKPiAgCWZl bmNlLT5zZXFubyA9IHNlcW5vOwo+ICAJZmVuY2UtPmZsYWdzID0gMFVMOwo+ICAJZmVuY2UtPmVy cm9yID0gMDsKPiArCWZlbmNlLT5vd25lciA9IG1vZHVsZTsKPiArCj4gKwlpZiAoIXRyeV9tb2R1 bGVfZ2V0KG1vZHVsZSkpCj4gKwkJZmVuY2UtPm93bmVyID0gTlVMTDsKPiAgCj4gIAl0cmFjZV9k bWFfZmVuY2VfaW5pdChmZW5jZSk7Cj4gIH0KPiAtRVhQT1JUX1NZTUJPTChkbWFfZmVuY2VfaW5p dCk7Cj4gK0VYUE9SVF9TWU1CT0woX2RtYV9mZW5jZV9pbml0KTsKCkRvIHdlIHN0aWxsIG5lZWQg dG8gZXhwb3J0IHRoZSBzeW1ib2wsIGl0IHdvbid0IGJlIGNhbGxlZCBmcm9tIG91dHNpZGUKYW55 bW9yZT8gT3RoZXIgdGhhbiB0aGF0IGxvb2tzIGdvb2QgdG8gbWU6CgpSZXZpZXdlZC1ieTogR3Vz dGF2byBQYWRvdmFuIDxndXN0YXZvLnBhZG92YW5AY29sbGFib3JhLmNvbT4KCkd1c3Rhdm8KCl9f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBt YWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3Rz LmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-qt0-f194.google.com ([209.85.216.194]:46346 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751325AbeFVKEW (ORCPT ); Fri, 22 Jun 2018 06:04:22 -0400 Message-ID: <1529661856.7034.404.camel@padovan.org> Subject: Re: [PATCH v2] dma-buf/fence: Take refcount on the module that owns the fence From: Gustavo Padovan To: Akhil P Oommen , sumit.semwal@linaro.org, jcrouse@codeaurora.org, linux-media@vger.kernel.org, dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, linux-kernel@vger.kernel.org Cc: smasetty@codeaurora.org, linux-arm-msm@vger.kernel.org Date: Fri, 22 Jun 2018 07:04:16 -0300 In-Reply-To: <1529660407-6266-1-git-send-email-akhilpo@codeaurora.org> References: <1529660407-6266-1-git-send-email-akhilpo@codeaurora.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: Hi Akhil, On Fri, 2018-06-22 at 15:10 +0530, Akhil P Oommen wrote: > Each fence object holds function pointers of the module that > initialized > it. Allowing the module to unload before this fence's release is > catastrophic. So, keep a refcount on the module until the fence is > released. > > Signed-off-by: Akhil P Oommen > --- > Changes in v2: > - added description for the new function parameter. > > drivers/dma-buf/dma-fence.c | 16 +++++++++++++--- > include/linux/dma-fence.h | 10 ++++++++-- > 2 files changed, 21 insertions(+), 5 deletions(-) > > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma- > fence.c > index 4edb9fd..2aaa44e 100644 > --- a/drivers/dma-buf/dma-fence.c > +++ b/drivers/dma-buf/dma-fence.c > @@ -18,6 +18,7 @@ > * more details. > */ > > +#include > #include > #include > #include > @@ -168,6 +169,7 @@ void dma_fence_release(struct kref *kref) > { > struct dma_fence *fence = > container_of(kref, struct dma_fence, refcount); > + struct module *module = fence->owner; > > trace_dma_fence_destroy(fence); > > @@ -178,6 +180,8 @@ void dma_fence_release(struct kref *kref) > fence->ops->release(fence); > else > dma_fence_free(fence); > + > + module_put(module); > } > EXPORT_SYMBOL(dma_fence_release); > > @@ -541,6 +545,7 @@ struct default_wait_cb { > > /** > * dma_fence_init - Initialize a custom fence. > + * @module: [in] the module that calls this API > * @fence: [in] the fence to initialize > * @ops: [in] the dma_fence_ops for operations on this > fence > * @lock: [in] the irqsafe spinlock to use for locking > this fence > @@ -556,8 +561,9 @@ struct default_wait_cb { > * to check which fence is later by simply using dma_fence_later. > */ > void > -dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops > *ops, > - spinlock_t *lock, u64 context, unsigned seqno) > +_dma_fence_init(struct module *module, struct dma_fence *fence, > + const struct dma_fence_ops *ops, spinlock_t *lock, > + u64 context, unsigned seqno) > { > BUG_ON(!lock); > BUG_ON(!ops || !ops->wait || !ops->enable_signaling || > @@ -571,7 +577,11 @@ struct default_wait_cb { > fence->seqno = seqno; > fence->flags = 0UL; > fence->error = 0; > + fence->owner = module; > + > + if (!try_module_get(module)) > + fence->owner = NULL; > > trace_dma_fence_init(fence); > } > -EXPORT_SYMBOL(dma_fence_init); > +EXPORT_SYMBOL(_dma_fence_init); Do we still need to export the symbol, it won't be called from outside anymore? Other than that looks good to me: Reviewed-by: Gustavo Padovan Gustavo