* Re: [PATCH 2/4] eisa: call put_device if device_register fails
[not found] ` <1386959996-7958-3-git-send-email-levex@linux.com>
@ 2013-12-13 19:04 ` Bjorn Helgaas
2013-12-13 19:08 ` Levente Kurusa
0 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2013-12-13 19:04 UTC (permalink / raw)
To: Levente Kurusa; +Cc: LKML
On Fri, Dec 13, 2013 at 07:39:54PM +0100, Levente Kurusa wrote:
> We need to give up the last reference to edev->dev, so
> we need to call put_device().
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Levente Kurusa <levex@linux.com>
I applied this and will push it through my PCI tree, since I don't think
anybody really takes care of EISA these days.
I looked at other callers of device_register(), and most of them look like
they have the same problem. Can you fix the others, too? This patch says
"2/4", so maybe you already are, but I can't find the other patches in the
series.
Bjorn
> ---
> drivers/eisa/eisa-bus.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
> index 272a3ec..8842cde 100644
> --- a/drivers/eisa/eisa-bus.c
> +++ b/drivers/eisa/eisa-bus.c
> @@ -232,8 +232,10 @@ static int __init eisa_init_device(struct eisa_root_device *root,
> static int __init eisa_register_device(struct eisa_device *edev)
> {
> int rc = device_register(&edev->dev);
> - if (rc)
> + if (rc) {
> + put_device(&edev->dev);
> return rc;
> + }
>
> rc = device_create_file(&edev->dev, &dev_attr_signature);
> if (rc)
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] eisa: call put_device if device_register fails
2013-12-13 19:04 ` [PATCH 2/4] eisa: call put_device if device_register fails Bjorn Helgaas
@ 2013-12-13 19:08 ` Levente Kurusa
0 siblings, 0 replies; 11+ messages in thread
From: Levente Kurusa @ 2013-12-13 19:08 UTC (permalink / raw)
To: Bjorn Helgaas; +Cc: LKML
On 12/13/2013 08:04 PM, Bjorn Helgaas wrote:
> On Fri, Dec 13, 2013 at 07:39:54PM +0100, Levente Kurusa wrote:
>> We need to give up the last reference to edev->dev, so
>> we need to call put_device().
>>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Signed-off-by: Levente Kurusa <levex@linux.com>
>
> I applied this and will push it through my PCI tree, since I don't think
> anybody really takes care of EISA these days.
>
> I looked at other callers of device_register(), and most of them look like
> they have the same problem. Can you fix the others, too? This patch says
> "2/4", so maybe you already are, but I can't find the other patches in the
> series.
Yes, the patchset didn't hit LKML due to me being a noob with git-send-email. :-(
I will fix them, just sent this to verify that I am doing it right. :-)
Thanks for applying!
--
Regards,
Levente Kurusa
^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1386959996-7958-2-git-send-email-levex@linux.com>]
* Re: [PATCH 1/4] net: phy: call put_device on device_register() failure
[not found] ` <1386959996-7958-2-git-send-email-levex@linux.com>
@ 2013-12-14 17:25 ` Greg Kroah-Hartman
0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2013-12-14 17:25 UTC (permalink / raw)
To: Levente Kurusa; +Cc: LKML, Grant Likely, Michał Mirosław, netdev
On Fri, Dec 13, 2013 at 07:39:53PM +0100, Levente Kurusa wrote:
> It is required to call put_device() if device_register() fails,
> so that we give up the last reference to the device. Calling put_device
> allows for mdiobus_release to be executed, kfreeing the bus.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Signed-off-by: Levente Kurusa <levex@linux.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1386959996-7958-4-git-send-email-levex@linux.com>]
* Re: [PATCH 3/4] backlight: lcd: call put_device if device_register fails
[not found] ` <1386959996-7958-4-git-send-email-levex@linux.com>
@ 2013-12-16 4:52 ` Jingoo Han
2014-01-07 1:42 ` Jingoo Han
1 sibling, 0 replies; 11+ messages in thread
From: Jingoo Han @ 2013-12-16 4:52 UTC (permalink / raw)
To: 'Levente Kurusa', 'Andrew Morton'
Cc: 'LKML', 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, 'Jingoo Han'
On Saturday, December 14, 2013 3:40 AM, Levente Kurusa wrote:
>
> Currently we kfree the container of the device which failed to register.
> This is wrong as the last reference is not given up with a put_device
> call. Also, now that we have put_device() callen, we no longer need
> the kfree as the new_ld->dev.release function will take care of kfreeing
> the associated memory.
>
> Signed-off-by: Levente Kurusa <levex@linux.com>
(+cc Andrew Morton)
Acked-by: Jingoo Han <jg1.han@samsung.com>
It looks good.
According to the comment of device_register, put_device()
should be used, instead of directly freeing.
./drivers/base/core.c
* 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.
*/
int device_register(struct device *dev)
{
device_initialize(dev);
return device_add(dev);
}
EXPORT_SYMBOL_GPL(device_register);
Levente Kurusa,
By the way, don't send the same mails three times, without any
reason. It is the waste of traffic. :-(
Best regards,
Jingoo Han
> ---
> drivers/video/backlight/lcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
> index 93cf15e..7de847d 100644
> --- a/drivers/video/backlight/lcd.c
> +++ b/drivers/video/backlight/lcd.c
> @@ -228,7 +228,7 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
>
> rc = device_register(&new_ld->dev);
> if (rc) {
> - kfree(new_ld);
> + put_device(&new_ld->dev);
> return ERR_PTR(rc);
> }
>
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] backlight: lcd: call put_device if device_register fails
@ 2013-12-16 4:52 ` Jingoo Han
0 siblings, 0 replies; 11+ messages in thread
From: Jingoo Han @ 2013-12-16 4:52 UTC (permalink / raw)
To: 'Levente Kurusa', 'Andrew Morton'
Cc: 'LKML', 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, 'Jingoo Han'
On Saturday, December 14, 2013 3:40 AM, Levente Kurusa wrote:
>
> Currently we kfree the container of the device which failed to register.
> This is wrong as the last reference is not given up with a put_device
> call. Also, now that we have put_device() callen, we no longer need
> the kfree as the new_ld->dev.release function will take care of kfreeing
> the associated memory.
>
> Signed-off-by: Levente Kurusa <levex@linux.com>
(+cc Andrew Morton)
Acked-by: Jingoo Han <jg1.han@samsung.com>
It looks good.
According to the comment of device_register, put_device()
should be used, instead of directly freeing.
./drivers/base/core.c
* 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.
*/
int device_register(struct device *dev)
{
device_initialize(dev);
return device_add(dev);
}
EXPORT_SYMBOL_GPL(device_register);
Levente Kurusa,
By the way, don't send the same mails three times, without any
reason. It is the waste of traffic. :-(
Best regards,
Jingoo Han
> ---
> drivers/video/backlight/lcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
> index 93cf15e..7de847d 100644
> --- a/drivers/video/backlight/lcd.c
> +++ b/drivers/video/backlight/lcd.c
> @@ -228,7 +228,7 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
>
> rc = device_register(&new_ld->dev);
> if (rc) {
> - kfree(new_ld);
> + put_device(&new_ld->dev);
> return ERR_PTR(rc);
> }
>
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] backlight: lcd: call put_device if device_register fails
2013-12-16 4:52 ` Jingoo Han
@ 2013-12-16 17:16 ` Levente Kurusa
-1 siblings, 0 replies; 11+ messages in thread
From: Levente Kurusa @ 2013-12-16 17:16 UTC (permalink / raw)
To: Jingoo Han, 'Andrew Morton'
Cc: 'LKML', 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen',
linux-fbdev
On 12/16/2013 05:52 AM, Jingoo Han wrote:
> On Saturday, December 14, 2013 3:40 AM, Levente Kurusa wrote:
>>
>> Currently we kfree the container of the device which failed to register.
>> This is wrong as the last reference is not given up with a put_device
>> call. Also, now that we have put_device() callen, we no longer need
>> the kfree as the new_ld->dev.release function will take care of kfreeing
>> the associated memory.
>>
>> Signed-off-by: Levente Kurusa <levex@linux.com>
>
> (+cc Andrew Morton)
>
> Acked-by: Jingoo Han <jg1.han@samsung.com>
>
> It looks good.
> According to the comment of device_register, put_device()
> should be used, instead of directly freeing.
Indeed, this is also mostly explained in [0/4]. Thanks for the Ack!
>
[...]
>
> Levente Kurusa,
> By the way, don't send the same mails three times, without any
> reason. It is the waste of traffic. :-(
>
Yea, sorry about that I messed up my git's smtp config and hence most of the
messages bounced off. It didn't even reach LKML. Sorry once more.
--
Regards,
Levente Kurusa
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] backlight: lcd: call put_device if device_register fails
@ 2013-12-16 17:16 ` Levente Kurusa
0 siblings, 0 replies; 11+ messages in thread
From: Levente Kurusa @ 2013-12-16 17:16 UTC (permalink / raw)
To: Jingoo Han, 'Andrew Morton'
Cc: 'LKML', 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen',
linux-fbdev
On 12/16/2013 05:52 AM, Jingoo Han wrote:
> On Saturday, December 14, 2013 3:40 AM, Levente Kurusa wrote:
>>
>> Currently we kfree the container of the device which failed to register.
>> This is wrong as the last reference is not given up with a put_device
>> call. Also, now that we have put_device() callen, we no longer need
>> the kfree as the new_ld->dev.release function will take care of kfreeing
>> the associated memory.
>>
>> Signed-off-by: Levente Kurusa <levex@linux.com>
>
> (+cc Andrew Morton)
>
> Acked-by: Jingoo Han <jg1.han@samsung.com>
>
> It looks good.
> According to the comment of device_register, put_device()
> should be used, instead of directly freeing.
Indeed, this is also mostly explained in [0/4]. Thanks for the Ack!
>
[...]
>
> Levente Kurusa,
> By the way, don't send the same mails three times, without any
> reason. It is the waste of traffic. :-(
>
Yea, sorry about that I messed up my git's smtp config and hence most of the
messages bounced off. It didn't even reach LKML. Sorry once more.
--
Regards,
Levente Kurusa
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/4] backlight: lcd: call put_device if device_register fails
[not found] ` <1386959996-7958-4-git-send-email-levex@linux.com>
@ 2014-01-07 1:42 ` Jingoo Han
2014-01-07 1:42 ` Jingoo Han
1 sibling, 0 replies; 11+ messages in thread
From: Jingoo Han @ 2014-01-07 1:42 UTC (permalink / raw)
To: 'Levente Kurusa'
Cc: 'LKML', 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, 'Jingoo Han',
'Andrew Morton'
On Saturday, December 14, 2013 3:40 AM, Levente Kurusa wrote:
>
> Currently we kfree the container of the device which failed to register.
> This is wrong as the last reference is not given up with a put_device
> call. Also, now that we have put_device() callen, we no longer need
> the kfree as the new_ld->dev.release function will take care of kfreeing
> the associated memory.
>
> Signed-off-by: Levente Kurusa <levex@linux.com>
> ---
> drivers/video/backlight/lcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
> index 93cf15e..7de847d 100644
> --- a/drivers/video/backlight/lcd.c
> +++ b/drivers/video/backlight/lcd.c
> @@ -228,7 +228,7 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
>
> rc = device_register(&new_ld->dev);
> if (rc) {
> - kfree(new_ld);
> + put_device(&new_ld->dev);
> return ERR_PTR(rc);
> }
(+cc Andrew Morton)
Hi Levente Kurusa,
Would you fix the same thing for 'backlight.c' file?
./drivers/video/backlight/backlight.c
struct backlight_device *backlight_device_register(const char *name,
.....
rc = device_register(&new_bd->dev);
if (rc) {
kfree(new_bd);
return ERR_PTR(rc);
}
Best regards,
Jingoo Han
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] backlight: lcd: call put_device if device_register fails
@ 2014-01-07 1:42 ` Jingoo Han
0 siblings, 0 replies; 11+ messages in thread
From: Jingoo Han @ 2014-01-07 1:42 UTC (permalink / raw)
To: 'Levente Kurusa'
Cc: 'LKML', 'Jean-Christophe Plagniol-Villard',
'Tomi Valkeinen', linux-fbdev, 'Jingoo Han',
'Andrew Morton'
On Saturday, December 14, 2013 3:40 AM, Levente Kurusa wrote:
>
> Currently we kfree the container of the device which failed to register.
> This is wrong as the last reference is not given up with a put_device
> call. Also, now that we have put_device() callen, we no longer need
> the kfree as the new_ld->dev.release function will take care of kfreeing
> the associated memory.
>
> Signed-off-by: Levente Kurusa <levex@linux.com>
> ---
> drivers/video/backlight/lcd.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
> index 93cf15e..7de847d 100644
> --- a/drivers/video/backlight/lcd.c
> +++ b/drivers/video/backlight/lcd.c
> @@ -228,7 +228,7 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
>
> rc = device_register(&new_ld->dev);
> if (rc) {
> - kfree(new_ld);
> + put_device(&new_ld->dev);
> return ERR_PTR(rc);
> }
(+cc Andrew Morton)
Hi Levente Kurusa,
Would you fix the same thing for 'backlight.c' file?
./drivers/video/backlight/backlight.c
struct backlight_device *backlight_device_register(const char *name,
.....
rc = device_register(&new_bd->dev);
if (rc) {
kfree(new_bd);
return ERR_PTR(rc);
}
Best regards,
Jingoo Han
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 0/4] treewide: add missing put_device calls
@ 2013-12-13 19:22 Levente Kurusa
2013-12-13 19:22 ` Levente Kurusa
0 siblings, 1 reply; 11+ messages in thread
From: Levente Kurusa @ 2013-12-13 19:22 UTC (permalink / raw)
To: LKML
Hi,
This is just the beginning of patchset-set that aims to fix possible
problems caused by not calling put_device() if device_register() fails.
The root cause for the need to call put_device() is that the underlying
kobject still has a reference count of 1. Thus, device.release() will not
be called and the device will just sit there waiting for a put_device().
Adding the put_device() also removes the need for the call to kfree() as most
release functions already call kfree() on the container of the device.
While these have not been experienced, they are potential issues and thus
they need to be fixed. Also, they are a few more files that have the same
kind of issue, those will be fixed if these are accepted.
(Sorry for the noise, I messed up my SMTP server so it didn't reach LKML)
The patchset consists of the following patches:
net: phy: call put_device on device_register() failure
eisa: call put_device if device_register fails
backlight: lcd: call put_device if device_register fails
w1: call put_device if device_register fails
diffstat as follows:
drivers/eisa/eisa-bus.c | 4 +++-
drivers/net/phy/mdio_bus.c | 1 +
drivers/video/backlight/lcd.c | 2 +-
drivers/w1/w1_int.c | 5 ++---
4 files changed, 7 insertions(+), 5 deletions(-)
--
Regards,
Levente Kurusa
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] backlight: lcd: call put_device if device_register fails
2013-12-13 19:22 [PATCH 0/4] treewide: add missing put_device calls Levente Kurusa
@ 2013-12-13 19:22 ` Levente Kurusa
0 siblings, 0 replies; 11+ messages in thread
From: Levente Kurusa @ 2013-12-13 19:22 UTC (permalink / raw)
To: LKML
Cc: Levente Kurusa, Jingoo Han, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev
Currently we kfree the container of the device which failed to register.
This is wrong as the last reference is not given up with a put_device
call. Also, now that we have put_device() callen, we no longer need
the kfree as the new_ld->dev.release function will take care of kfreeing
the associated memory.
Signed-off-by: Levente Kurusa <levex@linux.com>
---
drivers/video/backlight/lcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index 93cf15e..7de847d 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -228,7 +228,7 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
rc = device_register(&new_ld->dev);
if (rc) {
- kfree(new_ld);
+ put_device(&new_ld->dev);
return ERR_PTR(rc);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 3/4] backlight: lcd: call put_device if device_register fails
@ 2013-12-13 19:22 ` Levente Kurusa
0 siblings, 0 replies; 11+ messages in thread
From: Levente Kurusa @ 2013-12-13 19:22 UTC (permalink / raw)
To: LKML
Cc: Levente Kurusa, Jingoo Han, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev
Currently we kfree the container of the device which failed to register.
This is wrong as the last reference is not given up with a put_device
call. Also, now that we have put_device() callen, we no longer need
the kfree as the new_ld->dev.release function will take care of kfreeing
the associated memory.
Signed-off-by: Levente Kurusa <levex@linux.com>
---
drivers/video/backlight/lcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c
index 93cf15e..7de847d 100644
--- a/drivers/video/backlight/lcd.c
+++ b/drivers/video/backlight/lcd.c
@@ -228,7 +228,7 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent,
rc = device_register(&new_ld->dev);
if (rc) {
- kfree(new_ld);
+ put_device(&new_ld->dev);
return ERR_PTR(rc);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-01-07 1:42 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1386959996-7958-1-git-send-email-levex@linux.com>
[not found] ` <1386959996-7958-3-git-send-email-levex@linux.com>
2013-12-13 19:04 ` [PATCH 2/4] eisa: call put_device if device_register fails Bjorn Helgaas
2013-12-13 19:08 ` Levente Kurusa
[not found] ` <1386959996-7958-2-git-send-email-levex@linux.com>
2013-12-14 17:25 ` [PATCH 1/4] net: phy: call put_device on device_register() failure Greg Kroah-Hartman
[not found] ` <1386959996-7958-4-git-send-email-levex@linux.com>
2013-12-16 4:52 ` [PATCH 3/4] backlight: lcd: call put_device if device_register fails Jingoo Han
2013-12-16 4:52 ` Jingoo Han
2013-12-16 17:16 ` Levente Kurusa
2013-12-16 17:16 ` Levente Kurusa
2014-01-07 1:42 ` Jingoo Han
2014-01-07 1:42 ` Jingoo Han
2013-12-13 19:22 [PATCH 0/4] treewide: add missing put_device calls Levente Kurusa
2013-12-13 19:22 ` [PATCH 3/4] backlight: lcd: call put_device if device_register fails Levente Kurusa
2013-12-13 19:22 ` Levente Kurusa
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.