Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead
On Wed 15 May 2013 02:52:51 AM CST, Yinghai Lu wrote: On Tue, May 14, 2013 at 9:57 AM, Liu Jiang liu...@gmail.com wrote: On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote: On Tue, May 14, 2013 at 7:59 AM, Liu Jiang liu...@gmail.com wrote: On 05/14/2013 04:26 PM, Gu Zheng wrote: I suggest to use pci_release_dev() instead because it also needs to release OF related resources. I will update it in next version. diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index bc075a3..2ac6338 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1281,7 +1281,7 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus pci_set_of_node(dev); if (pci_setup_device(dev)) { - kfree(dev); + pci_release_dev(dev-dev); return NULL; no, should move pci_set_of_node calling into pci_setup_device. Yinghai I'm not sure whether we should call pci_set_of_node() for SR-IOV devices too, any suggestions here? or just move down pci_set_of_node after pci_setup_device? anyway that is another bug. Yinghai I'm not familiar with the OF logic and can't make sure whether pci_setup_device() has dependency on dev-of_node. Feel it's more safe to call pci_release_of_node() on failing path instead of tuning call-site of pci_set_of_node(). -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead
On Wed 15 May 2013 10:43:02 PM CST, Yinghai Lu wrote: On Wed, May 15, 2013 at 7:39 AM, Liu Jiang liu...@gmail.com wrote: On Wed 15 May 2013 02:52:51 AM CST, Yinghai Lu wrote: On Tue, May 14, 2013 at 9:57 AM, Liu Jiang liu...@gmail.com wrote: On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote: On Tue, May 14, 2013 at 7:59 AM, Liu Jiang liu...@gmail.com wrote: On 05/14/2013 04:26 PM, Gu Zheng wrote: I suggest to use pci_release_dev() instead because it also needs to release OF related resources. I will update it in next version. diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index bc075a3..2ac6338 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1281,7 +1281,7 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus pci_set_of_node(dev); if (pci_setup_device(dev)) { - kfree(dev); + pci_release_dev(dev-dev); return NULL; no, should move pci_set_of_node calling into pci_setup_device. Yinghai I'm not sure whether we should call pci_set_of_node() for SR-IOV devices too, any suggestions here? or just move down pci_set_of_node after pci_setup_device? anyway that is another bug. I'm not familiar with the OF logic and can't make sure whether pci_setup_device() has dependency on dev-of_node. Feel it's more safe to call pci_release_of_node() on failing path instead of tuning call-site of pci_set_of_node(). that is another bug, let of guy handle it. Yinghai Hi Yinghai, I don't know any OF exports, could you please help to CC some OF experts? Thanks, Gerry -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead
On Thu 16 May 2013 05:29:31 AM CST, Benjamin Herrenschmidt wrote: On Wed, 2013-05-15 at 22:46 +0800, Liu Jiang wrote: I don't know any OF exports, could you please help to CC some OF experts? I wrote that code I think. Sorry, I've missed the beginning of the thread, what is the problem ? Cheers, Ben. Hi, Just found a little memory leak issue that we should call pci_release_of_node() on error recovery path in function pci_scan_device(). pci_set_of_node(dev); if (pci_setup_device(dev)) { kfree(dev); return NULL; } Regards! Gerry -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead
On 05/14/2013 04:26 PM, Gu Zheng wrote: On 05/14/2013 01:23 AM, Yinghai Lu wrote: On Mon, May 13, 2013 at 9:08 AM, Jiang Liu liu...@gmail.com wrote: From: Gu Zheng guz.f...@cn.fujitsu.com diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 4f0bc0a..bc075a3 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1131,6 +1131,7 @@ static void pci_release_dev(struct device *dev) struct pci_dev *pci_dev; pci_dev = to_pci_dev(dev); + pci_bus_put(pci_dev-bus); pci_release_capabilities(pci_dev); pci_release_of_node(pci_dev); kfree(pci_dev); @@ -1269,11 +1270,10 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) if (!pci_bus_read_dev_vendor_id(bus, devfn, l, 60*1000)) return NULL; - dev = alloc_pci_dev(); + dev = pci_alloc_dev(bus); if (!dev) return NULL; - dev-bus = bus; dev-devfn = devfn; dev-vendor = l 0x; dev-device = (l 16) 0x; in pci_setup_device() fail path, it release the ref to that bus. Yes, you're right, we need to release the bus' ref if pci_setup_device() failed. Hi Zheng, I suggest to use pci_release_dev() instead because it also needs to release OF related resources. I will update it in next version. diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index bc075a3..2ac6338 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1281,7 +1281,7 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus pci_set_of_node(dev); if (pci_setup_device(dev)) { - kfree(dev); + pci_release_dev(dev-dev); return NULL; } hanks for your correction.:) Best regards, Gu Yinghai -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead
On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote: On Tue, May 14, 2013 at 7:59 AM, Liu Jiang liu...@gmail.com wrote: On 05/14/2013 04:26 PM, Gu Zheng wrote: I suggest to use pci_release_dev() instead because it also needs to release OF related resources. I will update it in next version. diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index bc075a3..2ac6338 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1281,7 +1281,7 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus pci_set_of_node(dev); if (pci_setup_device(dev)) { - kfree(dev); + pci_release_dev(dev-dev); return NULL; no, should move pci_set_of_node calling into pci_setup_device. Yinghai I'm not sure whether we should call pci_set_of_node() for SR-IOV devices too, any suggestions here? -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html