From: arvind.yadav.cs@gmail.com (arvindY)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] coresight: use put_device() instead of kfree()
Date: Tue, 27 Mar 2018 08:00:50 +0530 [thread overview]
Message-ID: <5AB9ACDA.9090207@gmail.com> (raw)
In-Reply-To: <CANLsYkymOF2oG7TmKbRTakYKw0ECN_DXfkX5b8WjmS0Lz0sfig@mail.gmail.com>
On Tuesday 27 March 2018 03:16 AM, Mathieu Poirier wrote:
> drivers/hwtracing/coresight/coresight.c
> On 18 March 2018 at 01:38, Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
>> Never directly free @dev after calling device_register(), even
>> if it returned an error. Always use put_device() to give up the
>> reference initialized.
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> ---
>> drivers/hwtracing/coresight/coresight.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
>> index 389c4ba..132dfbc 100644
>> --- a/drivers/hwtracing/coresight/coresight.c
>> +++ b/drivers/hwtracing/coresight/coresight.c
>> @@ -1026,8 +1026,10 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>> dev_set_name(&csdev->dev, "%s", desc->pdata->name);
>>
>> ret = device_register(&csdev->dev);
>> - if (ret)
>> - goto err_device_register;
>> + if (ret) {
>> + put_device(&csdev->dev);
>> + goto err_kzalloc_csdev;
>> + }
>>
>> mutex_lock(&coresight_mutex);
>>
>> @@ -1038,8 +1040,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>>
>> return csdev;
>>
>> -err_device_register:
>> - kfree(conns);
> Apologies for the late reply, I was travelling.
>
> I suggest to simply replace kfree() with put_device() in order to
> concentrate the error handling code in the same area and make sure
> that memory allocated for @conns and @refcnts is freed.
>
> Thanks,
> Mathieu
If you will see the comment for device_register(drivers/base/core.c)
there is mentioned that 'NOTE: _Never_ directly free @dev
after calling this function, even if it returned an error!
Always use put_device() to give up the reference initialized
in this function instead.
Here put_device() will decrement the last reference and then
free the memory by calling dev->release. Internally
put_device() -> kobject_put() -> kobject_cleanup() which is
responsible to call 'dev -> release' and also free other kobject
resources. we should always avoid kfree() if device_register()
returned an error. Otherwise it'll not do clean up of other
kobject resources.
~arvind
>> err_kzalloc_conns:
>> kfree(refcnts);
>> err_kzalloc_refcnts:
>> --
>> 2.7.4
>>
WARNING: multiple messages have this Message-ID (diff)
From: arvindY <arvind.yadav.cs@gmail.com>
To: Mathieu Poirier <mathieu.poirier@linaro.org>
Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] coresight: use put_device() instead of kfree()
Date: Tue, 27 Mar 2018 08:00:50 +0530 [thread overview]
Message-ID: <5AB9ACDA.9090207@gmail.com> (raw)
In-Reply-To: <CANLsYkymOF2oG7TmKbRTakYKw0ECN_DXfkX5b8WjmS0Lz0sfig@mail.gmail.com>
On Tuesday 27 March 2018 03:16 AM, Mathieu Poirier wrote:
> drivers/hwtracing/coresight/coresight.c
> On 18 March 2018 at 01:38, Arvind Yadav <arvind.yadav.cs@gmail.com> wrote:
>> Never directly free @dev after calling device_register(), even
>> if it returned an error. Always use put_device() to give up the
>> reference initialized.
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> ---
>> drivers/hwtracing/coresight/coresight.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
>> index 389c4ba..132dfbc 100644
>> --- a/drivers/hwtracing/coresight/coresight.c
>> +++ b/drivers/hwtracing/coresight/coresight.c
>> @@ -1026,8 +1026,10 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>> dev_set_name(&csdev->dev, "%s", desc->pdata->name);
>>
>> ret = device_register(&csdev->dev);
>> - if (ret)
>> - goto err_device_register;
>> + if (ret) {
>> + put_device(&csdev->dev);
>> + goto err_kzalloc_csdev;
>> + }
>>
>> mutex_lock(&coresight_mutex);
>>
>> @@ -1038,8 +1040,6 @@ struct coresight_device *coresight_register(struct coresight_desc *desc)
>>
>> return csdev;
>>
>> -err_device_register:
>> - kfree(conns);
> Apologies for the late reply, I was travelling.
>
> I suggest to simply replace kfree() with put_device() in order to
> concentrate the error handling code in the same area and make sure
> that memory allocated for @conns and @refcnts is freed.
>
> Thanks,
> Mathieu
If you will see the comment for device_register(drivers/base/core.c)
there is mentioned that 'NOTE: _Never_ directly free @dev
after calling this function, even if it returned an error!
Always use put_device() to give up the reference initialized
in this function instead.
Here put_device() will decrement the last reference and then
free the memory by calling dev->release. Internally
put_device() -> kobject_put() -> kobject_cleanup() which is
responsible to call 'dev -> release' and also free other kobject
resources. we should always avoid kfree() if device_register()
returned an error. Otherwise it'll not do clean up of other
kobject resources.
~arvind
>> err_kzalloc_conns:
>> kfree(refcnts);
>> err_kzalloc_refcnts:
>> --
>> 2.7.4
>>
next prev parent reply other threads:[~2018-03-27 2:30 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-18 7:38 [PATCH] coresight: use put_device() instead of kfree() Arvind Yadav
2018-03-18 7:38 ` Arvind Yadav
2018-03-26 21:46 ` Mathieu Poirier
2018-03-26 21:46 ` Mathieu Poirier
2018-03-27 2:30 ` arvindY [this message]
2018-03-27 2:30 ` arvindY
2018-03-27 16:07 ` Mathieu Poirier
2018-03-27 16:07 ` Mathieu Poirier
2018-03-27 16:28 ` arvindY
2018-03-27 16:28 ` arvindY
2018-03-27 17:13 ` Mathieu Poirier
2018-03-27 17:13 ` Mathieu Poirier
2018-03-28 15:41 ` Mathieu Poirier
2018-03-28 15:41 ` Mathieu Poirier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5AB9ACDA.9090207@gmail.com \
--to=arvind.yadav.cs@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.