[PATCH 32/38] pcie: add missing put_device call

[PATCH 32/38] pcie: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
Removed the kfree() as put_device will result in release_pcie_device being
called and hence the container of the device will be kfree'd.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/pci/pcie/portdrv_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 08d131f..80fb1f2 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -345,7 +345,7 @@ static int pcie_device_init(struct pci_dev *pdev, int service, int irq)
 
 	retval = device_register(device);
 	if (retval)
-		kfree(pcie);
+		put_device(device);
 	else
 		get_device(device);
 	return retval;
-- 
1.8.3.1

[PATCH 33/38] scsi: transport: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
Remove the kfree() as well, because the put_device() will result in
iscsi_endpoint_release being called and hence it will be kfree'd.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/scsi/scsi_transport_iscsi.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index 63a6ca4..ff20f35 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -219,8 +219,10 @@ iscsi_create_endpoint(int dd_size)
 	ep->dev.class = &iscsi_endpoint_class;
 	dev_set_name(&ep->dev, "ep-%llu", (unsigned long long) id);
 	err = device_register(&ep->dev);
-        if (err)
-                goto free_ep;
+        if (err) {
+        	put_device(&ep->dev);
+	        return NULL;
+	}
 
 	err = sysfs_create_group(&ep->dev.kobj, &iscsi_endpoint_group);
 	if (err)
@@ -233,10 +235,6 @@ iscsi_create_endpoint(int dd_size)
 unregister_dev:
 	device_unregister(&ep->dev);
 	return NULL;
-
-free_ep:
-	kfree(ep);
-	return NULL;
 }
 EXPORT_SYMBOL_GPL(iscsi_create_endpoint);
 
-- 
1.8.3.1

Re: [PATCH 33/38] scsi: transport: add missing put_device call

[James Bottomley]

On Thu, 2013-12-19 at 16:06 +0100, Levente Kurusa wrote:
> This is required so that we give up the last reference to the device.

This isn't true.

> Remove the kfree() as well, because the put_device() will result in
> iscsi_endpoint_release being called and hence it will be kfree'd.

There's no real point to this patch.  The use case where we own the
device absolutely up until the point we hand out references is well
established and there are a number of destroy paths running through SCSI
code which don't go via refcounting and this is one of them.  They're
almost universally on error legs on device bring up as this one is.

James

[PATCH 34/38] infiniband: core: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/infiniband/core/sysfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
index faad2ca..bf279ce 100644
--- a/drivers/infiniband/core/sysfs.c
+++ b/drivers/infiniband/core/sysfs.c
@@ -824,8 +824,10 @@ int ib_device_register_sysfs(struct ib_device *device,
 	INIT_LIST_HEAD(&device->port_list);
 
 	ret = device_register(class_dev);
-	if (ret)
+	if (ret) {
+		put_device(class_dev);
 		goto err;
+	}
 
 	for (i = 0; i < ARRAY_SIZE(ib_class_attributes); ++i) {
 		ret = device_create_file(class_dev, ib_class_attributes[i]);
-- 
1.8.3.1

[PATCH 35/38] media: bt8xx: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.
Remove the kfree() because the put_device() call will actually call
release_sub_device which in turn kfrees the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/media/pci/bt8xx/bttv-gpio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/bt8xx/bttv-gpio.c b/drivers/media/pci/bt8xx/bttv-gpio.c
index 922e823..3f364b7 100644
--- a/drivers/media/pci/bt8xx/bttv-gpio.c
+++ b/drivers/media/pci/bt8xx/bttv-gpio.c
@@ -98,7 +98,7 @@ int bttv_sub_add_device(struct bttv_core *core, char *name)
 
 	err = device_register(&sub->dev);
 	if (0 != err) {
-		kfree(sub);
+		put_device(&sub->dev);
 		return err;
 	}
 	pr_info("%d: add subdevice \"%s\"\n", core->nr, dev_name(&sub->dev));
-- 
1.8.3.1

[PATCH 36/38] dio: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/dio/dio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dio/dio.c b/drivers/dio/dio.c
index 55dd88d..5d4d755 100644
--- a/drivers/dio/dio.c
+++ b/drivers/dio/dio.c
@@ -186,6 +186,7 @@ static int __init dio_init(void)
 	error = device_register(&dio_bus.dev);
 	if (error) {
 		pr_err("DIO: Error registering dio_bus\n");
+		put_device(&dio_bus.dev);
 		return error;
 	}
 
-- 
1.8.3.1

[PATCH 37/38] uwb: umc-dev: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/uwb/umc-dev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/uwb/umc-dev.c b/drivers/uwb/umc-dev.c
index 4613c13..7b0b268 100644
--- a/drivers/uwb/umc-dev.c
+++ b/drivers/uwb/umc-dev.c
@@ -66,6 +66,7 @@ int umc_device_register(struct umc_dev *umc)
 	return 0;
 
 error_device_register:
+	put_device(&umc->dev);
 	release_resource(&umc->resource);
 error_request_resource:
 	return err;
-- 
1.8.3.1

[PATCH 38/38] bcma: add missing put_device call

[Levente Kurusa]

This is required so that we give up the last reference to the device.

Signed-off-by: Levente Kurusa <levex@linux.com>
---
 drivers/bcma/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
index e15430a..5a9f6bd 100644
--- a/drivers/bcma/main.c
+++ b/drivers/bcma/main.c
@@ -176,6 +176,7 @@ static int bcma_register_cores(struct bcma_bus *bus)
 			bcma_err(bus,
 				 "Could not register dev for core 0x%03X\n",
 				 core->id.id);
+			put_device(&core->dev);
 			continue;
 		}
 		core->dev_registered = true;
-- 
1.8.3.1

Re: [PATCH 32/38] pcie: add missing put_device call

[Bjorn Helgaas]

[+cc Greg, Yinghai]

On Thu, Dec 19, 2013 at 04:06:46PM +0100, Levente Kurusa wrote:
> This is required so that we give up the last reference to the device.
> Removed the kfree() as put_device will result in release_pcie_device being
> called and hence the container of the device will be kfree'd.
> 
> Signed-off-by: Levente Kurusa <levex@linux.com>

Thanks, I applied a slightly modified version of this to my pci/deletion
branch for v3.14.

I think the get_device() after device_register() succeeds and the
put_device() before device_unregister() are superfluous, so I propose the
series included below.  Any comments?

> ---
>  drivers/pci/pcie/portdrv_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
> index 08d131f..80fb1f2 100644
> --- a/drivers/pci/pcie/portdrv_core.c
> +++ b/drivers/pci/pcie/portdrv_core.c
> @@ -345,7 +345,7 @@ static int pcie_device_init(struct pci_dev *pdev, int service, int irq)
>  
>  	retval = device_register(device);
>  	if (retval)
> -		kfree(pcie);
> +		put_device(device);
>  	else
>  		get_device(device);
>  	return retval;


commit 8f3acca9acec1503f6b374faef2d1013cbf502af
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Dec 19 14:20:09 2013 -0700

    PCI/portdrv: Cleanup error paths
    
    Make the straightline path the normal no-error path.  Check for errors and
    return them directly, instead of checking for success and putting the
    normal path in an "if" body.
    
    No functional change.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 0b6e76604068..fc86d323fecc 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -344,11 +344,13 @@ static int pcie_device_init(struct pci_dev *pdev, int service, int irq)
 	device_enable_async_suspend(device);
 
 	retval = device_register(device);
-	if (retval)
+	if (retval) {
 		kfree(pcie);
-	else
-		get_device(device);
-	return retval;
+		return retval;
+	}
+
+	get_device(device);
+	return 0;
 }
 
 /**
@@ -498,12 +500,12 @@ static int pcie_port_probe_service(struct device *dev)
 
 	pciedev = to_pcie_device(dev);
 	status = driver->probe(pciedev);
-	if (!status) {
-		dev_printk(KERN_DEBUG, dev, "service driver %s loaded\n",
-			driver->name);
-		get_device(dev);
-	}
-	return status;
+	if (status)
+		return status;
+
+	dev_printk(KERN_DEBUG, dev, "service driver %s loaded\n", driver->name);
+	get_device(dev);
+	return 0;
 }
 
 /**

commit f39862058e1f278e0495cd9ea57de571e74aa1fe
Author: Levente Kurusa <levex@linux.com>
Date:   Thu Dec 19 14:22:35 2013 -0700

    PCI/portdrv: Add put_device() after device_register() failure
    
    This is required so that we give up the last reference to the device.
    Removed the kfree() as put_device will result in release_pcie_device()
    being called and hence the container of the device will be kfree'd.
    
    [bhelgaas: fix conflict after my previous cleanup]
    Signed-off-by: Levente Kurusa <levex@linux.com>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index fc86d323fecc..9811eea53511 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -345,7 +345,7 @@ static int pcie_device_init(struct pci_dev *pdev, int service, int irq)
 
 	retval = device_register(device);
 	if (retval) {
-		kfree(pcie);
+		put_device(device);
 		return retval;
 	}
 

commit e75f34ce6633549486a044d64b2a79240d4113a8
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Thu Dec 19 14:24:13 2013 -0700

    PCI/portdrv: Remove extra get_device()/put_device() for pcie_device
    
    Previously pcie_device_init() called get_device() if device_register() for
    the new pcie_device succeeded, and remove_iter() called put_device() when
    removing before unregistering the device.
    
    But device_register() already increments the reference count in
    device_add(), so we don't need to do it again here.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
index 9811eea53511..6a6e54909335 100644
--- a/drivers/pci/pcie/portdrv_core.c
+++ b/drivers/pci/pcie/portdrv_core.c
@@ -349,7 +349,6 @@ static int pcie_device_init(struct pci_dev *pdev, int service, int irq)
 		return retval;
 	}
 
-	get_device(device);
 	return 0;
 }
 
@@ -456,10 +455,8 @@ int pcie_port_device_resume(struct device *dev)
 
 static int remove_iter(struct device *dev, void *data)
 {
-	if (dev->bus == &pcie_port_bus_type) {
-		put_device(dev);
+	if (dev->bus == &pcie_port_bus_type)
 		device_unregister(dev);
-	}
 	return 0;
 }
 

Re: [PATCH 32/38] pcie: add missing put_device call

[Greg Kroah-Hartman]

On Thu, Dec 19, 2013 at 03:10:19PM -0700, Bjorn Helgaas wrote:
> [+cc Greg, Yinghai]
> 
> On Thu, Dec 19, 2013 at 04:06:46PM +0100, Levente Kurusa wrote:
> > This is required so that we give up the last reference to the device.
> > Removed the kfree() as put_device will result in release_pcie_device being
> > called and hence the container of the device will be kfree'd.
> > 
> > Signed-off-by: Levente Kurusa <levex@linux.com>
> 
> Thanks, I applied a slightly modified version of this to my pci/deletion
> branch for v3.14.
> 
> I think the get_device() after device_register() succeeds and the
> put_device() before device_unregister() are superfluous, so I propose the
> series included below.  Any comments?

Looks good to me.

greg k-h

Re: [PATCH 32/38] pcie: add missing put_device call

[Yinghai Lu]

On Thu, Dec 19, 2013 at 2:10 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [+cc Greg, Yinghai]
>
> On Thu, Dec 19, 2013 at 04:06:46PM +0100, Levente Kurusa wrote:
>> This is required so that we give up the last reference to the device.
>> Removed the kfree() as put_device will result in release_pcie_device being
>> called and hence the container of the device will be kfree'd.
>>
>> Signed-off-by: Levente Kurusa <levex@linux.com>
>
> Thanks, I applied a slightly modified version of this to my pci/deletion
> branch for v3.14.
>
> I think the get_device() after device_register() succeeds and the
> put_device() before device_unregister() are superfluous, so I propose the
> series included below.  Any comments?

Nice cleanup.

Thanks

Yinghai