[+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 <le...@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 <bhelg...@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 <bhelg...@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 <le...@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 <le...@linux.com>
    Signed-off-by: Bjorn Helgaas <bhelg...@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 <bhelg...@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 <bhelg...@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;
 }
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to