From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joonas Lahtinen Subject: Re: [Intel-gfx] [PATCH] dma-buf: Release module reference on creation failure Date: Thu, 31 Mar 2016 19:20:16 +0300 Message-ID: <1459441216.8191.21.camel@linux.intel.com> References: <1459413350-31082-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1459413350-31082-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Chris Wilson , intel-gfx@lists.freedesktop.org Cc: linaro-mm-sig@lists.linaro.org, Daniel Vetter , dri-devel@lists.freedesktop.org, linux-media@vger.kernel.org List-Id: dri-devel@lists.freedesktop.org T24gdG8sIDIwMTYtMDMtMzEgYXQgMDk6MzUgKzAxMDAsIENocmlzIFdpbHNvbiB3cm90ZToKPiBJ ZiB3ZSBmYWlsIHRvIGNyZWF0ZSB0aGUgYW5vbiBmaWxlLCB3ZSBuZWVkIHRvIHJlbWVtYmVyIHRv IHJlbGVhc2UgdGhlCj4gbW9kdWxlIHJlZmVyZW5jZSBvbiB0aGUgb3duZXIuCj4gCj4gU2lnbmVk LW9mZi1ieTogQ2hyaXMgV2lsc29uIDxjaHJpc0BjaHJpcy13aWxzb24uY28udWs+Cj4gQ2M6IFN1 bWl0IFNlbXdhbCA8c3VtaXQuc2Vtd2FsQGxpbmFyby5vcmc+Cj4gQ2M6IERhbmllbCBWZXR0ZXIg PGRhbmllbC52ZXR0ZXJAZmZ3bGwuY2g+Cj4gQ2M6IGxpbnV4LW1lZGlhQHZnZXIua2VybmVsLm9y Zwo+IENjOiBkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCj4gQ2M6IGxpbmFyby1tbS1z aWdAbGlzdHMubGluYXJvLm9yZwo+IC0tLQo+IMKgZHJpdmVycy9kbWEtYnVmL2RtYS1idWYuYyB8 IDE1ICsrKysrKysrKysrLS0tLQo+IMKgMSBmaWxlIGNoYW5nZWQsIDExIGluc2VydGlvbnMoKyks IDQgZGVsZXRpb25zKC0pCj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZG1hLWJ1Zi9kbWEtYnVm LmMgYi9kcml2ZXJzL2RtYS1idWYvZG1hLWJ1Zi5jCj4gaW5kZXggNGEyYzA3ZWU2Njc3Li42ZjBm MGIxMGEyNDEgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9kbWEtYnVmL2RtYS1idWYuYwo+ICsrKyBi L2RyaXZlcnMvZG1hLWJ1Zi9kbWEtYnVmLmMKPiBAQCAtMzMzLDYgKzMzMyw3IEBAIHN0cnVjdCBk bWFfYnVmICpkbWFfYnVmX2V4cG9ydChjb25zdCBzdHJ1Y3QgZG1hX2J1Zl9leHBvcnRfaW5mbyAq ZXhwX2luZm8pCj4gwqAJc3RydWN0IHJlc2VydmF0aW9uX29iamVjdCAqcmVzdiA9IGV4cF9pbmZv LT5yZXN2Owo+IMKgCXN0cnVjdCBmaWxlICpmaWxlOwo+IMKgCXNpemVfdCBhbGxvY19zaXplID0g c2l6ZW9mKHN0cnVjdCBkbWFfYnVmKTsKPiArCWludCByZXQ7Cj4gwqAKPiDCoAlpZiAoIWV4cF9p bmZvLT5yZXN2KQo+IMKgCQlhbGxvY19zaXplICs9IHNpemVvZihzdHJ1Y3QgcmVzZXJ2YXRpb25f b2JqZWN0KTsKPiBAQCAtMzU2LDggKzM1Nyw4IEBAIHN0cnVjdCBkbWFfYnVmICpkbWFfYnVmX2V4 cG9ydChjb25zdCBzdHJ1Y3QgZG1hX2J1Zl9leHBvcnRfaW5mbyAqZXhwX2luZm8pCj4gwqAKPiDC oAlkbWFidWYgPSBremFsbG9jKGFsbG9jX3NpemUsIEdGUF9LRVJORUwpOwo+IMKgCWlmICghZG1h YnVmKSB7Cj4gLQkJbW9kdWxlX3B1dChleHBfaW5mby0+b3duZXIpOwo+IC0JCXJldHVybiBFUlJf UFRSKC1FTk9NRU0pOwo+ICsJCXJldCA9IC1FTk9NRU07Cj4gKwkJZ290byBmcmVlX21vZHVsZTsK PiDCoAl9Cj4gwqAKPiDCoAlkbWFidWYtPnByaXYgPSBleHBfaW5mby0+cHJpdjsKPiBAQCAtMzc4 LDggKzM3OSw4IEBAIHN0cnVjdCBkbWFfYnVmICpkbWFfYnVmX2V4cG9ydChjb25zdCBzdHJ1Y3Qg ZG1hX2J1Zl9leHBvcnRfaW5mbyAqZXhwX2luZm8pCj4gwqAJZmlsZSA9IGFub25faW5vZGVfZ2V0 ZmlsZSgiZG1hYnVmIiwgJmRtYV9idWZfZm9wcywgZG1hYnVmLAo+IMKgCQkJCQlleHBfaW5mby0+ ZmxhZ3MpOwo+IMKgCWlmIChJU19FUlIoZmlsZSkpIHsKPiAtCQlrZnJlZShkbWFidWYpOwo+IC0J CXJldHVybiBFUlJfQ0FTVChmaWxlKTsKPiArCQlyZXQgPSBQVFJfRVJSKGZpbGUpOwo+ICsJCWdv dG8gZnJlZV9kbWFidWY7Cj4gwqAJfQo+IMKgCj4gwqAJZmlsZS0+Zl9tb2RlIHw9IEZNT0RFX0xT RUVLOwo+IEBAIC0zOTMsNiArMzk0LDEyIEBAIHN0cnVjdCBkbWFfYnVmICpkbWFfYnVmX2V4cG9y dChjb25zdCBzdHJ1Y3QgZG1hX2J1Zl9leHBvcnRfaW5mbyAqZXhwX2luZm8pCj4gwqAJbXV0ZXhf dW5sb2NrKCZkYl9saXN0LmxvY2spOwo+IMKgCj4gwqAJcmV0dXJuIGRtYWJ1ZjsKPiArCj4gK2Zy ZWVfZG1hYnVmOgo+ICsJa2ZyZWUoZG1hYnVmKTsKPiArZnJlZV9tb2R1bGU6Cj4gKwltb2R1bGVf cHV0KGV4cF9pbmZvLT5vd25lcik7Cj4gKwlyZXR1cm4gRVJSX1BUUihyZXQpOwoKTGFiZWxzIGNv dWxkIHJlYWxseSBiZSBlcnJfZG1hYnVmICgvIG91dF9kbWFidWYpLiBUaGF0J3MgbW9yZSBpbiBs aW5lCndpdGggcmVzdCBvZiB0aGUgY29kZWJhc2UgYW5kIGtlcm5lbCBjb2Rpbmcgc3R5bGU6ICdB biBleGFtcGxlIG9mIGEKZ29vZCBuYW1lIGNvdWxkIGJlICJvdXRfYnVmZmVyOiIgaWYgdGhlIGdv dG8gZnJlZXMgImJ1ZmZlciIuJwoKImZyZWVfZG1hYnVmIiBkb2VzIHNvdW5kIHRvIG1lIGxpa2Ug aXQgY291bGQgYWxzbyBiZSBleGVjdXRlZCBvbiB0aGUKbm9ybWFsIGV4ZWN1dGlvbiBwYXRoIG9m IHRoZSBmdW5jdGlvbi4KCk90aGVyIHRoYW4gdGhhdCwKClJldmlld2VkLWJ5OiBKb29uYXMgTGFo dGluZW4gPGpvb25hcy5sYWh0aW5lbkBsaW51eC5pbnRlbC5jb20+Cgo+IMKgfQo+IMKgRVhQT1JU X1NZTUJPTF9HUEwoZG1hX2J1Zl9leHBvcnQpOwo+IMKgCi0tIApKb29uYXMgTGFodGluZW4KT3Bl biBTb3VyY2UgVGVjaG5vbG9neSBDZW50ZXIKSW50ZWwgQ29ycG9yYXRpb24KX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlz dApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0 b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mga09.intel.com ([134.134.136.24]:57568 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751609AbcCaQ0z (ORCPT ); Thu, 31 Mar 2016 12:26:55 -0400 Message-ID: <1459441216.8191.21.camel@linux.intel.com> Subject: Re: [Intel-gfx] [PATCH] dma-buf: Release module reference on creation failure From: Joonas Lahtinen To: Chris Wilson , intel-gfx@lists.freedesktop.org Cc: Daniel Vetter , dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org, Sumit Semwal , linux-media@vger.kernel.org Date: Thu, 31 Mar 2016 19:20:16 +0300 In-Reply-To: <1459413350-31082-1-git-send-email-chris@chris-wilson.co.uk> References: <1459413350-31082-1-git-send-email-chris@chris-wilson.co.uk> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-media-owner@vger.kernel.org List-ID: On to, 2016-03-31 at 09:35 +0100, Chris Wilson wrote: > If we fail to create the anon file, we need to remember to release the > module reference on the owner. > > Signed-off-by: Chris Wilson > Cc: Sumit Semwal > Cc: Daniel Vetter > Cc: linux-media@vger.kernel.org > Cc: dri-devel@lists.freedesktop.org > Cc: linaro-mm-sig@lists.linaro.org > --- >  drivers/dma-buf/dma-buf.c | 15 +++++++++++---- >  1 file changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > index 4a2c07ee6677..6f0f0b10a241 100644 > --- a/drivers/dma-buf/dma-buf.c > +++ b/drivers/dma-buf/dma-buf.c > @@ -333,6 +333,7 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) >   struct reservation_object *resv = exp_info->resv; >   struct file *file; >   size_t alloc_size = sizeof(struct dma_buf); > + int ret; >   >   if (!exp_info->resv) >   alloc_size += sizeof(struct reservation_object); > @@ -356,8 +357,8 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) >   >   dmabuf = kzalloc(alloc_size, GFP_KERNEL); >   if (!dmabuf) { > - module_put(exp_info->owner); > - return ERR_PTR(-ENOMEM); > + ret = -ENOMEM; > + goto free_module; >   } >   >   dmabuf->priv = exp_info->priv; > @@ -378,8 +379,8 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) >   file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, >   exp_info->flags); >   if (IS_ERR(file)) { > - kfree(dmabuf); > - return ERR_CAST(file); > + ret = PTR_ERR(file); > + goto free_dmabuf; >   } >   >   file->f_mode |= FMODE_LSEEK; > @@ -393,6 +394,12 @@ struct dma_buf *dma_buf_export(const struct dma_buf_export_info *exp_info) >   mutex_unlock(&db_list.lock); >   >   return dmabuf; > + > +free_dmabuf: > + kfree(dmabuf); > +free_module: > + module_put(exp_info->owner); > + return ERR_PTR(ret); Labels could really be err_dmabuf (/ out_dmabuf). That's more in line with rest of the codebase and kernel coding style: 'An example of a good name could be "out_buffer:" if the goto frees "buffer".' "free_dmabuf" does sound to me like it could also be executed on the normal execution path of the function. Other than that, Reviewed-by: Joonas Lahtinen >  } >  EXPORT_SYMBOL_GPL(dma_buf_export); >   -- Joonas Lahtinen Open Source Technology Center Intel Corporation