Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-28 Thread Mathieu Poirier 
On 18 March 2018 at 01:38, Arvind Yadav  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 
> ---
>  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(>dev, "%s", desc->pdata->name);
>
> ret = device_register(>dev);
> -   if (ret)
> -   goto err_device_register;
> +   if (ret) {
> +   put_device(>dev);
> +   goto err_kzalloc_csdev;
> +   }
>
> mutex_lock(_mutex);
>
> @@ -1038,8 +1040,6 @@ struct coresight_device *coresight_register(struct 
> coresight_desc *desc)
>
> return csdev;
>
> -err_device_register:
> -   kfree(conns);
>  err_kzalloc_conns:
> kfree(refcnts);
>  err_kzalloc_refcnts:
> --
> 2.7.4
>

Applied - thanks
Mathieu

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-28 Thread Mathieu Poirier 
On 18 March 2018 at 01:38, Arvind Yadav  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 
> ---
>  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(>dev, "%s", desc->pdata->name);
>
> ret = device_register(>dev);
> -   if (ret)
> -   goto err_device_register;
> +   if (ret) {
> +   put_device(>dev);
> +   goto err_kzalloc_csdev;
> +   }
>
> mutex_lock(_mutex);
>
> @@ -1038,8 +1040,6 @@ struct coresight_device *coresight_register(struct 
> coresight_desc *desc)
>
> return csdev;
>
> -err_device_register:
> -   kfree(conns);
>  err_kzalloc_conns:
> kfree(refcnts);
>  err_kzalloc_refcnts:
> --
> 2.7.4
>

Applied - thanks
Mathieu

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-27 Thread Mathieu Poirier 
On 27 March 2018 at 10:28, arvindY  wrote:
>
>
> On Tuesday 27 March 2018 09:37 PM, Mathieu Poirier wrote:
>>
>> On 26 March 2018 at 20:30, arvindY  wrote:
>>>
>>>
>>> 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 
 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 
> ---
>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(>dev, "%s", desc->pdata->name);
>
>   ret = device_register(>dev);
> -   if (ret)
> -   goto err_device_register;
> +   if (ret) {
> +   put_device(>dev);
> +   goto err_kzalloc_csdev;
> +   }
>
>   mutex_lock(_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.
>>
>> I have read the notice and in full agreement with the put_device() part.
>>
>>> 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.
>>
>> Memory would be automatically freed if it would have been allocated
>> with the devm_XYZ() helpers but it is not the case here.  As such it
>> has to be freed explicitly.  Reading your patch again (and the jetlag
>> somewhat fading) I think you've done the right thing except for the
>> "goto" that should still point to "err_device_register".
>>
>
> Take rest :)
> 'goto' should not point to "err_device_register" because
> put_device() will call 'dev->release' which is coresight_device_release().
> It's release @conns, @refcnt and @csdev . If we will keep same 'goto
> then kfree() will be redundant for all.

You are correct - your patch does exactly what should be done.

>
>
>>> 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
>
>

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-27 Thread Mathieu Poirier 
On 27 March 2018 at 10:28, arvindY  wrote:
>
>
> On Tuesday 27 March 2018 09:37 PM, Mathieu Poirier wrote:
>>
>> On 26 March 2018 at 20:30, arvindY  wrote:
>>>
>>>
>>> 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 
 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 
> ---
>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(>dev, "%s", desc->pdata->name);
>
>   ret = device_register(>dev);
> -   if (ret)
> -   goto err_device_register;
> +   if (ret) {
> +   put_device(>dev);
> +   goto err_kzalloc_csdev;
> +   }
>
>   mutex_lock(_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.
>>
>> I have read the notice and in full agreement with the put_device() part.
>>
>>> 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.
>>
>> Memory would be automatically freed if it would have been allocated
>> with the devm_XYZ() helpers but it is not the case here.  As such it
>> has to be freed explicitly.  Reading your patch again (and the jetlag
>> somewhat fading) I think you've done the right thing except for the
>> "goto" that should still point to "err_device_register".
>>
>
> Take rest :)
> 'goto' should not point to "err_device_register" because
> put_device() will call 'dev->release' which is coresight_device_release().
> It's release @conns, @refcnt and @csdev . If we will keep same 'goto
> then kfree() will be redundant for all.

You are correct - your patch does exactly what should be done.

>
>
>>> 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
>
>

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-27 Thread arvindY 



On Tuesday 27 March 2018 09:37 PM, Mathieu Poirier wrote:

On 26 March 2018 at 20:30, arvindY  wrote:


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  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 
---
   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(>dev, "%s", desc->pdata->name);

  ret = device_register(>dev);
-   if (ret)
-   goto err_device_register;
+   if (ret) {
+   put_device(>dev);
+   goto err_kzalloc_csdev;
+   }

  mutex_lock(_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.

I have read the notice and in full agreement with the put_device() part.


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.

Memory would be automatically freed if it would have been allocated
with the devm_XYZ() helpers but it is not the case here.  As such it
has to be freed explicitly.  Reading your patch again (and the jetlag
somewhat fading) I think you've done the right thing except for the
"goto" that should still point to "err_device_register".



Take rest :)
'goto' should not point to "err_device_register" because
put_device() will call 'dev->release' which is coresight_device_release().
It's release @conns, @refcnt and @csdev . If we will keep same 'goto
then kfree() will be redundant for all.



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

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-27 Thread arvindY 



On Tuesday 27 March 2018 09:37 PM, Mathieu Poirier wrote:

On 26 March 2018 at 20:30, arvindY  wrote:


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  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 
---
   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(>dev, "%s", desc->pdata->name);

  ret = device_register(>dev);
-   if (ret)
-   goto err_device_register;
+   if (ret) {
+   put_device(>dev);
+   goto err_kzalloc_csdev;
+   }

  mutex_lock(_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.

I have read the notice and in full agreement with the put_device() part.


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.

Memory would be automatically freed if it would have been allocated
with the devm_XYZ() helpers but it is not the case here.  As such it
has to be freed explicitly.  Reading your patch again (and the jetlag
somewhat fading) I think you've done the right thing except for the
"goto" that should still point to "err_device_register".



Take rest :)
'goto' should not point to "err_device_register" because
put_device() will call 'dev->release' which is coresight_device_release().
It's release @conns, @refcnt and @csdev . If we will keep same 'goto
then kfree() will be redundant for all.



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

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-27 Thread Mathieu Poirier 
On 26 March 2018 at 20:30, arvindY  wrote:
>
>
> 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  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 
>>> ---
>>>   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(>dev, "%s", desc->pdata->name);
>>>
>>>  ret = device_register(>dev);
>>> -   if (ret)
>>> -   goto err_device_register;
>>> +   if (ret) {
>>> +   put_device(>dev);
>>> +   goto err_kzalloc_csdev;
>>> +   }
>>>
>>>  mutex_lock(_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.

I have read the notice and in full agreement with the put_device() part.

> 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.

Memory would be automatically freed if it would have been allocated
with the devm_XYZ() helpers but it is not the case here.  As such it
has to be freed explicitly.  Reading your patch again (and the jetlag
somewhat fading) I think you've done the right thing except for the
"goto" that should still point to "err_device_register".

>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
>>>
>

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-27 Thread Mathieu Poirier 
On 26 March 2018 at 20:30, arvindY  wrote:
>
>
> 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  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 
>>> ---
>>>   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(>dev, "%s", desc->pdata->name);
>>>
>>>  ret = device_register(>dev);
>>> -   if (ret)
>>> -   goto err_device_register;
>>> +   if (ret) {
>>> +   put_device(>dev);
>>> +   goto err_kzalloc_csdev;
>>> +   }
>>>
>>>  mutex_lock(_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.

I have read the notice and in full agreement with the put_device() part.

> 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.

Memory would be automatically freed if it would have been allocated
with the devm_XYZ() helpers but it is not the case here.  As such it
has to be freed explicitly.  Reading your patch again (and the jetlag
somewhat fading) I think you've done the right thing except for the
"goto" that should still point to "err_device_register".

>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
>>>
>

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-26 Thread arvindY 



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  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 
---
  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(>dev, "%s", desc->pdata->name);

 ret = device_register(>dev);
-   if (ret)
-   goto err_device_register;
+   if (ret) {
+   put_device(>dev);
+   goto err_kzalloc_csdev;
+   }

 mutex_lock(_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

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-26 Thread arvindY 



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  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 
---
  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(>dev, "%s", desc->pdata->name);

 ret = device_register(>dev);
-   if (ret)
-   goto err_device_register;
+   if (ret) {
+   put_device(>dev);
+   goto err_kzalloc_csdev;
+   }

 mutex_lock(_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

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-26 Thread Mathieu Poirier 
drivers/hwtracing/coresight/coresight.c
On 18 March 2018 at 01:38, Arvind Yadav  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 
> ---
>  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(>dev, "%s", desc->pdata->name);
>
> ret = device_register(>dev);
> -   if (ret)
> -   goto err_device_register;
> +   if (ret) {
> +   put_device(>dev);
> +   goto err_kzalloc_csdev;
> +   }
>
> mutex_lock(_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

>  err_kzalloc_conns:
> kfree(refcnts);
>  err_kzalloc_refcnts:
> --
> 2.7.4
>

Re: [PATCH] coresight: use put_device() instead of kfree()

2018-03-26 Thread Mathieu Poirier 
drivers/hwtracing/coresight/coresight.c
On 18 March 2018 at 01:38, Arvind Yadav  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 
> ---
>  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(>dev, "%s", desc->pdata->name);
>
> ret = device_register(>dev);
> -   if (ret)
> -   goto err_device_register;
> +   if (ret) {
> +   put_device(>dev);
> +   goto err_kzalloc_csdev;
> +   }
>
> mutex_lock(_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

>  err_kzalloc_conns:
> kfree(refcnts);
>  err_kzalloc_refcnts:
> --
> 2.7.4
>

[PATCH] coresight: use put_device() instead of kfree()

2018-03-18 Thread Arvind Yadav 
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 
---
 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(>dev, "%s", desc->pdata->name);
 
ret = device_register(>dev);
-   if (ret)
-   goto err_device_register;
+   if (ret) {
+   put_device(>dev);
+   goto err_kzalloc_csdev;
+   }
 
mutex_lock(_mutex);
 
@@ -1038,8 +1040,6 @@ struct coresight_device *coresight_register(struct 
coresight_desc *desc)
 
return csdev;
 
-err_device_register:
-   kfree(conns);
 err_kzalloc_conns:
kfree(refcnts);
 err_kzalloc_refcnts:
-- 
2.7.4

[PATCH] coresight: use put_device() instead of kfree()

2018-03-18 Thread Arvind Yadav 
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 
---
 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(>dev, "%s", desc->pdata->name);
 
ret = device_register(>dev);
-   if (ret)
-   goto err_device_register;
+   if (ret) {
+   put_device(>dev);
+   goto err_kzalloc_csdev;
+   }
 
mutex_lock(_mutex);
 
@@ -1038,8 +1040,6 @@ struct coresight_device *coresight_register(struct 
coresight_desc *desc)
 
return csdev;
 
-err_device_register:
-   kfree(conns);
 err_kzalloc_conns:
kfree(refcnts);
 err_kzalloc_refcnts:
-- 
2.7.4