* Re: [PATCH 32/38] pcie: add missing put_device call
2013-12-19 15:06 [PATCH 32/38] pcie: add missing put_device call Levente Kurusa
@ 2013-12-19 22:10 ` Bjorn Helgaas
2013-12-19 22:34 ` Greg Kroah-Hartman
2013-12-20 6:15 ` Yinghai Lu
0 siblings, 2 replies; 4+ messages in thread
From: Bjorn Helgaas @ 2013-12-19 22:10 UTC (permalink / raw)
To: Levente Kurusa
Cc: LKML, Andrew Murray, Myron Stowe, linux-pci, Greg Kroah-Hartman,
Yinghai Lu
[+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;
}
^ permalink raw reply related [flat|nested] 4+ messages in thread