* [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead [not found] <1368461313-4371-1-git-send-email-jiang.liu@huawei.com> @ 2013-05-13 16:08 ` Jiang Liu 2013-05-13 17:23 ` Yinghai Lu 0 siblings, 1 reply; 15+ messages in thread From: Jiang Liu @ 2013-05-13 16:08 UTC (permalink / raw) To: Bjorn Helgaas, Yinghai Lu Cc: Neela Syam Kolli, sparclinux, Toshi Kani, Jiang Liu, linux-scsi, David Airlie, Greg Kroah-Hartman, linuxppc-dev, linux-kernel, James E.J. Bottomley, Rafael J . Wysocki, Yijing Wang, linux-pci, Gu Zheng, Paul Mackerras, Andrew Morton, Myron Stowe, David S. Miller, Jiang Liu From: Gu Zheng <guz.fnst@cn.fujitsu.com> <marker to prevent gmail from removing below "From:"> From: Gu Zheng <guz.fnst@cn.fujitsu.com> Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void). v2: Follow Bjorn's correction to move pci_bus_put() to pci_release_dev() instead. Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org> Cc: Paul Mackerras <paulus@samba.org> Cc: "David S. Miller" <davem@davemloft.net> Cc: David Airlie <airlied@linux.ie> Cc: Neela Syam Kolli <megaraidlinux@lsi.com> Cc: "James E.J. Bottomley" <JBottomley@parallels.com> Cc: Yinghai Lu <yinghai@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-kernel@vger.kernel.org Cc: sparclinux@vger.kernel.org Cc: linux-pci@vger.kernel.org Cc: linux-scsi@vger.kernel.org Signed-off-by: Jiang Liu <jiang.liu@huawei.com> --- arch/powerpc/kernel/pci_of_scan.c | 3 +-- arch/sparc/kernel/pci.c | 3 +-- drivers/char/agp/alpha-agp.c | 2 +- drivers/char/agp/parisc-agp.c | 2 +- drivers/pci/iov.c | 8 +++++--- drivers/pci/probe.c | 4 ++-- drivers/scsi/megaraid.c | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c index 2a67e9b..24d01c4 100644 --- a/arch/powerpc/kernel/pci_of_scan.c +++ b/arch/powerpc/kernel/pci_of_scan.c @@ -128,7 +128,7 @@ struct pci_dev *of_create_pci_dev(struct device_node *node, const char *type; struct pci_slot *slot; - dev = alloc_pci_dev(); + dev = pci_alloc_dev(bus); if (!dev) return NULL; type = of_get_property(node, "device_type", NULL); @@ -137,7 +137,6 @@ struct pci_dev *of_create_pci_dev(struct device_node *node, pr_debug(" create device, devfn: %x, type: %s\n", devfn, type); - dev->bus = bus; dev->dev.of_node = of_node_get(node); dev->dev.parent = bus->bridge; dev->dev.bus = &pci_bus_type; diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c index baf4366..e5871fb 100644 --- a/arch/sparc/kernel/pci.c +++ b/arch/sparc/kernel/pci.c @@ -254,7 +254,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm, const char *type; u32 class; - dev = alloc_pci_dev(); + dev = pci_alloc_dev(bus); if (!dev) return NULL; @@ -281,7 +281,6 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm, printk(" create device, devfn: %x, type: %s\n", devfn, type); - dev->bus = bus; dev->sysdata = node; dev->dev.parent = bus->bridge; dev->dev.bus = &pci_bus_type; diff --git a/drivers/char/agp/alpha-agp.c b/drivers/char/agp/alpha-agp.c index dd84af4..199b8e9 100644 --- a/drivers/char/agp/alpha-agp.c +++ b/drivers/char/agp/alpha-agp.c @@ -174,7 +174,7 @@ alpha_core_agp_setup(void) /* * Build a fake pci_dev struct */ - pdev = alloc_pci_dev(); + pdev = pci_alloc_dev(NULL); if (!pdev) return -ENOMEM; pdev->vendor = 0xffff; diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c index 94821ab..bf5d247 100644 --- a/drivers/char/agp/parisc-agp.c +++ b/drivers/char/agp/parisc-agp.c @@ -333,7 +333,7 @@ parisc_agp_setup(void __iomem *ioc_hpa, void __iomem *lba_hpa) struct agp_bridge_data *bridge; int error = 0; - fake_bridge_dev = alloc_pci_dev(); + fake_bridge_dev = pci_alloc_dev(NULL); if (!fake_bridge_dev) { error = -ENOMEM; goto fail; diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index ee599f2..24134cd 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -75,18 +75,20 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset) struct pci_dev *virtfn; struct resource *res; struct pci_sriov *iov = dev->sriov; + struct pci_bus *bus; - virtfn = alloc_pci_dev(); + virtfn = pci_alloc_dev(NULL); if (!virtfn) return -ENOMEM; mutex_lock(&iov->dev->sriov->lock); - virtfn->bus = virtfn_add_bus(dev->bus, virtfn_bus(dev, id)); - if (!virtfn->bus) { + bus = virtfn_add_bus(dev->bus, virtfn_bus(dev, id)); + if (!bus) { kfree(virtfn); mutex_unlock(&iov->dev->sriov->lock); return -ENOMEM; } + virtfn->bus = pci_bus_get(bus); virtfn->devfn = virtfn_devfn(dev, id); virtfn->vendor = dev->vendor; pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device); 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 & 0xffff; dev->device = (l >> 16) & 0xffff; diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c index 9504ec0..e1660ca 100644 --- a/drivers/scsi/megaraid.c +++ b/drivers/scsi/megaraid.c @@ -2025,7 +2025,7 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor) static inline int make_local_pdev(adapter_t *adapter, struct pci_dev **pdev) { - *pdev = alloc_pci_dev(); + *pdev = pci_alloc_dev(NULL); if( *pdev == NULL ) return -1; -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-13 16:08 ` [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead Jiang Liu @ 2013-05-13 17:23 ` Yinghai Lu 2013-05-14 8:26 ` Gu Zheng 0 siblings, 1 reply; 15+ messages in thread From: Yinghai Lu @ 2013-05-13 17:23 UTC (permalink / raw) To: Jiang Liu Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Jiang Liu, Linux-Scsi, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Gu Zheng, Yijing Wang, linux-pci@vger.kernel.org, Bjorn Helgaas, Paul Mackerras, Andrew Morton, Myron Stowe, David S. Miller On Mon, May 13, 2013 at 9:08 AM, Jiang Liu <liuj97@gmail.com> wrote: > From: Gu Zheng <guz.fnst@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 & 0xffff; > dev->device = (l >> 16) & 0xffff; in pci_setup_device() fail path, it release the ref to that bus. Yinghai ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-13 17:23 ` Yinghai Lu @ 2013-05-14 8:26 ` Gu Zheng 2013-05-14 14:59 ` Liu Jiang 0 siblings, 1 reply; 15+ messages in thread From: Gu Zheng @ 2013-05-14 8:26 UTC (permalink / raw) To: Yinghai Lu Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Jiang Liu, Linux-Scsi, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Yijing Wang, linux-pci@vger.kernel.org, Bjorn Helgaas, Paul Mackerras, Andrew Morton, Myron Stowe, David S. Miller, Jiang Liu [-- Attachment #1: Type: text/plain, Size: 1297 bytes --] On 05/14/2013 01:23 AM, Yinghai Lu wrote: > On Mon, May 13, 2013 at 9:08 AM, Jiang Liu <liuj97@gmail.com> wrote: >> From: Gu Zheng <guz.fnst@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 & 0xffff; >> dev->device = (l >> 16) & 0xffff; > > 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. Thanks for your correction.:) Best regards, Gu > > Yinghai > [-- Attachment #2: Convert-alloc_pci_dev-void-to-pci_alloc_dev-v3.patch --] [-- Type: text/plain, Size: 5295 bytes --] >From 7add6d9e70919b95be2debde2f58fc31d26c75bf Mon Sep 17 00:00:00 2001 From: Gu Zheng <guz.fnst@cn.fujitsu.com> Date: Tue, 14 May 2013 16:11:07 +0800 Subject: [PATCH v3] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead v3: Follow Yinghai's correction to release the bus' ref in pci_setup_device() fail path. v2: Follow Bjorn's correction to move pci_bus_put() to pci_release_dev() instead. Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com> --- arch/powerpc/kernel/pci_of_scan.c | 3 +-- arch/sparc/kernel/pci.c | 3 +-- drivers/char/agp/alpha-agp.c | 2 +- drivers/char/agp/parisc-agp.c | 2 +- drivers/pci/iov.c | 8 +++++--- drivers/pci/probe.c | 5 +++-- drivers/scsi/megaraid.c | 2 +- 7 files changed, 13 insertions(+), 12 deletions(-) diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c index 2a67e9b..24d01c4 100644 --- a/arch/powerpc/kernel/pci_of_scan.c +++ b/arch/powerpc/kernel/pci_of_scan.c @@ -128,7 +128,7 @@ struct pci_dev *of_create_pci_dev(struct device_node *node, const char *type; struct pci_slot *slot; - dev = alloc_pci_dev(); + dev = pci_alloc_dev(bus); if (!dev) return NULL; type = of_get_property(node, "device_type", NULL); @@ -137,7 +137,6 @@ struct pci_dev *of_create_pci_dev(struct device_node *node, pr_debug(" create device, devfn: %x, type: %s\n", devfn, type); - dev->bus = bus; dev->dev.of_node = of_node_get(node); dev->dev.parent = bus->bridge; dev->dev.bus = &pci_bus_type; diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c index baf4366..e5871fb 100644 --- a/arch/sparc/kernel/pci.c +++ b/arch/sparc/kernel/pci.c @@ -254,7 +254,7 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm, const char *type; u32 class; - dev = alloc_pci_dev(); + dev = pci_alloc_dev(bus); if (!dev) return NULL; @@ -281,7 +281,6 @@ static struct pci_dev *of_create_pci_dev(struct pci_pbm_info *pbm, printk(" create device, devfn: %x, type: %s\n", devfn, type); - dev->bus = bus; dev->sysdata = node; dev->dev.parent = bus->bridge; dev->dev.bus = &pci_bus_type; diff --git a/drivers/char/agp/alpha-agp.c b/drivers/char/agp/alpha-agp.c index dd84af4..199b8e9 100644 --- a/drivers/char/agp/alpha-agp.c +++ b/drivers/char/agp/alpha-agp.c @@ -174,7 +174,7 @@ alpha_core_agp_setup(void) /* * Build a fake pci_dev struct */ - pdev = alloc_pci_dev(); + pdev = pci_alloc_dev(NULL); if (!pdev) return -ENOMEM; pdev->vendor = 0xffff; diff --git a/drivers/char/agp/parisc-agp.c b/drivers/char/agp/parisc-agp.c index 94821ab..bf5d247 100644 --- a/drivers/char/agp/parisc-agp.c +++ b/drivers/char/agp/parisc-agp.c @@ -333,7 +333,7 @@ parisc_agp_setup(void __iomem *ioc_hpa, void __iomem *lba_hpa) struct agp_bridge_data *bridge; int error = 0; - fake_bridge_dev = alloc_pci_dev(); + fake_bridge_dev = pci_alloc_dev(NULL); if (!fake_bridge_dev) { error = -ENOMEM; goto fail; diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c index c93071d..2652ca0 100644 --- a/drivers/pci/iov.c +++ b/drivers/pci/iov.c @@ -75,18 +75,20 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset) struct pci_dev *virtfn; struct resource *res; struct pci_sriov *iov = dev->sriov; + struct pci_bus *bus; - virtfn = alloc_pci_dev(); + virtfn = pci_alloc_dev(NULL); if (!virtfn) return -ENOMEM; mutex_lock(&iov->dev->sriov->lock); - virtfn->bus = virtfn_add_bus(dev->bus, virtfn_bus(dev, id)); - if (!virtfn->bus) { + bus = virtfn_add_bus(dev->bus, virtfn_bus(dev, id)); + if (!bus) { kfree(virtfn); mutex_unlock(&iov->dev->sriov->lock); return -ENOMEM; } + virtfn->bus = pci_bus_get(bus); virtfn->devfn = virtfn_devfn(dev, id); virtfn->vendor = dev->vendor; pci_read_config_word(dev, iov->pos + PCI_SRIOV_VF_DID, &virtfn->device); diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 70f10fa..db8dadc 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1130,6 +1130,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); @@ -1263,11 +1264,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 & 0xffff; dev->device = (l >> 16) & 0xffff; @@ -1275,6 +1275,7 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) pci_set_of_node(dev); if (pci_setup_device(dev)) { + pci_bus_put(bus); kfree(dev); return NULL; } diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c index 846f475f..90c95a3 100644 --- a/drivers/scsi/megaraid.c +++ b/drivers/scsi/megaraid.c @@ -2026,7 +2026,7 @@ megaraid_abort_and_reset(adapter_t *adapter, Scsi_Cmnd *cmd, int aor) static inline int make_local_pdev(adapter_t *adapter, struct pci_dev **pdev) { - *pdev = alloc_pci_dev(); + *pdev = pci_alloc_dev(NULL); if( *pdev == NULL ) return -1; -- 1.7.7 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-14 8:26 ` Gu Zheng @ 2013-05-14 14:59 ` Liu Jiang 2013-05-14 15:10 ` Yinghai Lu 0 siblings, 1 reply; 15+ messages in thread From: Liu Jiang @ 2013-05-14 14:59 UTC (permalink / raw) To: Gu Zheng Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Jiang Liu, Linux-Scsi, David Airlie, Myron Stowe, linuxppc-dev, linux-pci@vger.kernel.org, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Yijing Wang, Greg Kroah-Hartman, Bjorn Helgaas, Paul Mackerras, Andrew Morton, Yinghai Lu, David S. Miller 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 <liuj97@gmail.com> wrote: >>> From: Gu Zheng <guz.fnst@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 & 0xffff; >>> dev->device = (l >> 16) & 0xffff; >> 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 >> > ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-14 14:59 ` Liu Jiang @ 2013-05-14 15:10 ` Yinghai Lu 2013-05-14 16:57 ` Liu Jiang 0 siblings, 1 reply; 15+ messages in thread From: Yinghai Lu @ 2013-05-14 15:10 UTC (permalink / raw) To: Liu Jiang Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Jiang Liu, Linux-Scsi, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Bjorn Helgaas, Yijing Wang, linux-pci@vger.kernel.org, Gu Zheng, Paul Mackerras, Andrew Morton, Myron Stowe, David S. Miller On Tue, May 14, 2013 at 7:59 AM, Liu Jiang <liuj97@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-14 15:10 ` Yinghai Lu @ 2013-05-14 16:57 ` Liu Jiang 2013-05-14 18:52 ` Yinghai Lu 0 siblings, 1 reply; 15+ messages in thread From: Liu Jiang @ 2013-05-14 16:57 UTC (permalink / raw) To: Yinghai Lu Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Jiang Liu, Linux-Scsi, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Bjorn Helgaas, Yijing Wang, linux-pci@vger.kernel.org, Gu Zheng, Paul Mackerras, Andrew Morton, Myron Stowe, David S. Miller On Tue 14 May 2013 11:10:33 PM CST, Yinghai Lu wrote: > On Tue, May 14, 2013 at 7:59 AM, Liu Jiang <liuj97@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? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-14 16:57 ` Liu Jiang @ 2013-05-14 18:52 ` Yinghai Lu 2013-05-15 14:39 ` Liu Jiang 0 siblings, 1 reply; 15+ messages in thread From: Yinghai Lu @ 2013-05-14 18:52 UTC (permalink / raw) To: Liu Jiang Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Jiang Liu, Linux-Scsi, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Bjorn Helgaas, Yijing Wang, linux-pci@vger.kernel.org, Gu Zheng, Paul Mackerras, Andrew Morton, Myron Stowe, David S. Miller On Tue, May 14, 2013 at 9:57 AM, Liu Jiang <liuj97@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 <liuj97@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-14 18:52 ` Yinghai Lu @ 2013-05-15 14:39 ` Liu Jiang 2013-05-15 14:43 ` Yinghai Lu 0 siblings, 1 reply; 15+ messages in thread From: Liu Jiang @ 2013-05-15 14:39 UTC (permalink / raw) To: Yinghai Lu Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Jiang Liu, Linux-Scsi, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Bjorn Helgaas, Yijing Wang, linux-pci@vger.kernel.org, Gu Zheng, Paul Mackerras, Andrew Morton, Myron Stowe, David S. Miller On Wed 15 May 2013 02:52:51 AM CST, Yinghai Lu wrote: > On Tue, May 14, 2013 at 9:57 AM, Liu Jiang <liuj97@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 <liuj97@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(). ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-15 14:39 ` Liu Jiang @ 2013-05-15 14:43 ` Yinghai Lu 2013-05-15 14:46 ` Liu Jiang 0 siblings, 1 reply; 15+ messages in thread From: Yinghai Lu @ 2013-05-15 14:43 UTC (permalink / raw) To: Liu Jiang Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Jiang Liu, Linux-Scsi, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Bjorn Helgaas, Yijing Wang, linux-pci@vger.kernel.org, Gu Zheng, Paul Mackerras, Andrew Morton, Myron Stowe, David S. Miller On Wed, May 15, 2013 at 7:39 AM, Liu Jiang <liuj97@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 <liuj97@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 <liuj97@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-15 14:43 ` Yinghai Lu @ 2013-05-15 14:46 ` Liu Jiang 2013-05-15 14:58 ` Yinghai Lu 2013-05-15 21:29 ` Benjamin Herrenschmidt 0 siblings, 2 replies; 15+ messages in thread From: Liu Jiang @ 2013-05-15 14:46 UTC (permalink / raw) To: Yinghai Lu Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Jiang Liu, Linux-Scsi, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Bjorn Helgaas, Yijing Wang, linux-pci@vger.kernel.org, Gu Zheng, Paul Mackerras, Andrew Morton, Myron Stowe, David S. Miller On Wed 15 May 2013 10:43:02 PM CST, Yinghai Lu wrote: > On Wed, May 15, 2013 at 7:39 AM, Liu Jiang <liuj97@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 <liuj97@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 <liuj97@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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-15 14:46 ` Liu Jiang @ 2013-05-15 14:58 ` Yinghai Lu 2013-05-15 21:32 ` Benjamin Herrenschmidt 2013-05-15 21:29 ` Benjamin Herrenschmidt 1 sibling, 1 reply; 15+ messages in thread From: Yinghai Lu @ 2013-05-15 14:58 UTC (permalink / raw) To: Liu Jiang, Benjamin Herrenschmidt, Rafael J . Wysocki, linuxppc-dev, sparclinux@vger.kernel.org, David S. Miller Cc: linux-pci@vger.kernel.org, Linux Kernel Mailing List, Bjorn Helgaas, Paul Mackerras, Greg Kroah-Hartman, Gu Zheng, Myron Stowe On Wed, May 15, 2013 at 7:46 AM, Liu Jiang <liuj97@gmail.com> wrote: > On Wed 15 May 2013 10:43:02 PM CST, Yinghai Lu wrote: >> > >> 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? powerpc and sparc are using that. Ben, in drivers/pci/probe.c::pci_scan_device() there is pci_set_of_node(dev); if (pci_setup_device(dev)) { kfree(dev); return NULL; } so if pci_setup_device fails, there is one dev reference is not release. please check you can just move down pci_set_of_node down after that failing path, like if (pci_setup_device(dev)) { kfree(dev); return NULL; } pci_set_of_node(dev); Yinghai ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-15 14:58 ` Yinghai Lu @ 2013-05-15 21:32 ` Benjamin Herrenschmidt 2013-05-15 21:52 ` Yinghai Lu 0 siblings, 1 reply; 15+ messages in thread From: Benjamin Herrenschmidt @ 2013-05-15 21:32 UTC (permalink / raw) To: Yinghai Lu Cc: Bjorn Helgaas, Myron Stowe, Greg Kroah-Hartman, Linux Kernel Mailing List, Rafael J . Wysocki, Gu Zheng, Paul Mackerras, linux-pci@vger.kernel.org, sparclinux@vger.kernel.org, linuxppc-dev, David S. Miller, Liu Jiang On Wed, 2013-05-15 at 07:58 -0700, Yinghai Lu wrote: > Ben, > > in drivers/pci/probe.c::pci_scan_device() there is > > pci_set_of_node(dev); > > if (pci_setup_device(dev)) { > kfree(dev); > return NULL; > } > > so if pci_setup_device fails, there is one dev reference is not release. > > please check you can just move down pci_set_of_node down after that > failing path, like > > > if (pci_setup_device(dev)) { > kfree(dev); > return NULL; > } > > pci_set_of_node(dev); No, we want the OF node set when we run the quirks, we intentionally do that early, the right thing to do is to to call pci_release_of_node() in the error path (it's safe to call even if the node is NULL). Cheers, Ben. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-15 21:32 ` Benjamin Herrenschmidt @ 2013-05-15 21:52 ` Yinghai Lu 0 siblings, 0 replies; 15+ messages in thread From: Yinghai Lu @ 2013-05-15 21:52 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Bjorn Helgaas, Myron Stowe, Greg Kroah-Hartman, Linux Kernel Mailing List, Rafael J . Wysocki, Gu Zheng, Paul Mackerras, linux-pci@vger.kernel.org, sparclinux@vger.kernel.org, linuxppc-dev, David S. Miller, Liu Jiang On Wed, May 15, 2013 at 2:32 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > On Wed, 2013-05-15 at 07:58 -0700, Yinghai Lu wrote: > >> Ben, >> >> in drivers/pci/probe.c::pci_scan_device() there is >> >> pci_set_of_node(dev); >> >> if (pci_setup_device(dev)) { >> kfree(dev); >> return NULL; >> } >> >> so if pci_setup_device fails, there is one dev reference is not release. >> >> please check you can just move down pci_set_of_node down after that >> failing path, like >> >> >> if (pci_setup_device(dev)) { >> kfree(dev); >> return NULL; >> } >> >> pci_set_of_node(dev); > > No, we want the OF node set when we run the quirks, we intentionally do > that early, the right thing to do is to to call pci_release_of_node() > in the error path (it's safe to call even if the node is NULL). > Good. We have two options. 1. can you please submit one complete patch, and get it merged into v3.10. 2. put it together with pci_alloc_dev patchset towards to v3.11? Thanks Yinghai ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-15 14:46 ` Liu Jiang 2013-05-15 14:58 ` Yinghai Lu @ 2013-05-15 21:29 ` Benjamin Herrenschmidt 2013-05-15 23:46 ` Liu Jiang 1 sibling, 1 reply; 15+ messages in thread From: Benjamin Herrenschmidt @ 2013-05-15 21:29 UTC (permalink / raw) To: Liu Jiang Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Linux-Scsi, Myron Stowe, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Bjorn Helgaas, Yijing Wang, Paul Mackerras, linux-pci@vger.kernel.org, Gu Zheng, Andrew Morton, Yinghai Lu, David S. Miller, Jiang Liu 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. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead 2013-05-15 21:29 ` Benjamin Herrenschmidt @ 2013-05-15 23:46 ` Liu Jiang 0 siblings, 0 replies; 15+ messages in thread From: Liu Jiang @ 2013-05-15 23:46 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Neela Syam Kolli, sparclinux@vger.kernel.org, Toshi Kani, Linux-Scsi, Myron Stowe, David Airlie, Greg Kroah-Hartman, linuxppc-dev, Linux Kernel Mailing List, James E.J. Bottomley, Rafael J . Wysocki, Bjorn Helgaas, Yijing Wang, Paul Mackerras, linux-pci@vger.kernel.org, Gu Zheng, Andrew Morton, Yinghai Lu, David S. Miller, Jiang Liu 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-05-15 23:47 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1368461313-4371-1-git-send-email-jiang.liu@huawei.com>
2013-05-13 16:08 ` [PATCH v2, part 1 3/9] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead Jiang Liu
2013-05-13 17:23 ` Yinghai Lu
2013-05-14 8:26 ` Gu Zheng
2013-05-14 14:59 ` Liu Jiang
2013-05-14 15:10 ` Yinghai Lu
2013-05-14 16:57 ` Liu Jiang
2013-05-14 18:52 ` Yinghai Lu
2013-05-15 14:39 ` Liu Jiang
2013-05-15 14:43 ` Yinghai Lu
2013-05-15 14:46 ` Liu Jiang
2013-05-15 14:58 ` Yinghai Lu
2013-05-15 21:32 ` Benjamin Herrenschmidt
2013-05-15 21:52 ` Yinghai Lu
2013-05-15 21:29 ` Benjamin Herrenschmidt
2013-05-15 23:46 ` Liu Jiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).