Re: [PATCH] net: hns: use put_device() if device_register fail

[arvindY]

On Monday 12 March 2018 10:59 PM, Richard Weinberger wrote:

On Mon, Mar 12, 2018 at 5:27 PM, arvindY  wrote:


On Monday 12 March 2018 08:13 PM, David Miller wrote:

From: Arvind Yadav 
Date: Fri,  9 Mar 2018 16:11:17 +0530


if device_register() returned an error! Always use put_device()
to give up the reference initialized.

Signed-off-by: Arvind Yadav 

I do not see anything giving cls_dev an initial non-zero reference
count before this device_register() call.

Yes,  you are correct there is nothing to release (hnae_release).

Wait, this is not what DaveM said.
Since you sent also patches to MTD I care about this too.

Do we have to call put_device() in any case after a failure of
device_register() or not?
In this case the release function is empty, so nothing is going to released.
But technically a put_device() is needed and correct according to your
change log.

I have to admit I don't know all details of the driver core, maybe you
can help me.
device_register() calls device_initialize() followed by device_add().
device_initialize() does a kobject_init() which again sets the
reference counter to 1 via kref_init().
In the next step device_add() does a get_device() --> reference
counter goes up to 2.
But in the exit path of the function a put_device() is done, also in
case of an error.
So device_register() always returns with reference count 1.

If I understand this correctly a put_device() is mandatory.
Also in this driver.
Can you please enlighten me? :-)


Oh, I just miss understood david question.

But what ever you are telling that is correct.
device_initialize() will call kref_init() which is setting
refcount 1 by refcount_set(>refcount, 1).
and device_add() will call kref_get() to increment refcount
by calling refcount_inc(>refcount).
So we need put_device() to decrement and release the
kboject.
Internally it'll call kref_put() -> refcount_dec_and_test(>refcount)
to decrement refcount.

~arvind

Re: [PATCH] net: hns: use put_device() if device_register fail

[Richard Weinberger]

On Mon, Mar 12, 2018 at 5:27 PM, arvindY  wrote:
>
>
> On Monday 12 March 2018 08:13 PM, David Miller wrote:
>>
>> From: Arvind Yadav 
>> Date: Fri,  9 Mar 2018 16:11:17 +0530
>>
>>> if device_register() returned an error! Always use put_device()
>>> to give up the reference initialized.
>>>
>>> Signed-off-by: Arvind Yadav 
>>
>> I do not see anything giving cls_dev an initial non-zero reference
>> count before this device_register() call.
>
> Yes,  you are correct there is nothing to release (hnae_release).

Wait, this is not what DaveM said.
Since you sent also patches to MTD I care about this too.

Do we have to call put_device() in any case after a failure of
device_register() or not?
In this case the release function is empty, so nothing is going to released.
But technically a put_device() is needed and correct according to your
change log.

I have to admit I don't know all details of the driver core, maybe you
can help me.
device_register() calls device_initialize() followed by device_add().
device_initialize() does a kobject_init() which again sets the
reference counter to 1 via kref_init().
In the next step device_add() does a get_device() --> reference
counter goes up to 2.
But in the exit path of the function a put_device() is done, also in
case of an error.
So device_register() always returns with reference count 1.

If I understand this correctly a put_device() is mandatory.
Also in this driver.
Can you please enlighten me? :-)

-- 
Thanks,
//richard

Re: [PATCH] net: hns: use put_device() if device_register fail

[arvindY]

On Monday 12 March 2018 08:13 PM, David Miller wrote:

From: Arvind Yadav 
Date: Fri,  9 Mar 2018 16:11:17 +0530


if device_register() returned an error! Always use put_device()
to give up the reference initialized.

Signed-off-by: Arvind Yadav 

I do not see anything giving cls_dev an initial non-zero reference
count before this device_register() call.

Yes,  you are correct there is nothing to release (hnae_release).


And I have no idea why you use a "!" when saying 'error' you this
commit log message.


Sorry for that. next time I will take care.

~arvind

Re: [PATCH] net: hns: use put_device() if device_register fail

[David Miller]

From: Arvind Yadav 
Date: Fri,  9 Mar 2018 16:11:17 +0530

> if device_register() returned an error! Always use put_device()
> to give up the reference initialized.
> 
> Signed-off-by: Arvind Yadav 

I do not see anything giving cls_dev an initial non-zero reference
count before this device_register() call.

And I have no idea why you use a "!" when saying 'error' you this
commit log message.

[PATCH] net: hns: use put_device() if device_register fail

[Arvind Yadav]

if device_register() returned an error! Always use put_device()
to give up the reference initialized.

Signed-off-by: Arvind Yadav 
---
 drivers/net/ethernet/hisilicon/hns/hnae.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hnae.c 
b/drivers/net/ethernet/hisilicon/hns/hnae.c
index a051e58..0cf5ceb 100644
--- a/drivers/net/ethernet/hisilicon/hns/hnae.c
+++ b/drivers/net/ethernet/hisilicon/hns/hnae.c
@@ -422,8 +422,10 @@ int hnae_ae_register(struct hnae_ae_dev *hdev, struct 
module *owner)
hdev->cls_dev.release = hnae_release;
(void)dev_set_name(>cls_dev, "hnae%d", hdev->id);
ret = device_register(>cls_dev);
-   if (ret)
+   if (ret) {
+   put_device(>cls_dev);
return ret;
+   }
 
__module_get(THIS_MODULE);
 
-- 
1.9.1