[PATCH 2/3] PCI: rename alloc_pci_dev() to pci_alloc_dev()

[PATCH 2/3] PCI: rename alloc_pci_dev() to pci_alloc_dev()

[Gu Zheng]

>From a3135f4a814ec2faba007f030ca96ea2297484ef Mon Sep 17 00:00:00 2001
From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Thu, 18 Apr 2013 17:27:40 +0900
Subject: [PATCH 2/3] PCI: rename alloc_pci_dev() to pci_alloc_dev()

Rename alloc_pci_dev() to pci_alloc_dev() to make the name style the same
as pci_alloc_bus(), pci_destroy_dev().

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Reviewed-by: Li Guang <lig.fnst@cn.fujitsu.com>
---
 arch/powerpc/kernel/pci_of_scan.c |    2 +-
 drivers/char/agp/alpha-agp.c      |    2 +-
 drivers/char/agp/parisc-agp.c     |    2 +-
 drivers/pci/iov.c                 |    2 +-
 drivers/pci/probe.c               |    6 +++---
 drivers/scsi/megaraid.c           |    2 +-
 include/linux/pci.h               |    2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 9e0f6fd..2e35d11 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();
 	if (!dev)
 		return NULL;
 	type = of_get_property(node, "device_type", NULL);
diff --git a/drivers/char/agp/alpha-agp.c b/drivers/char/agp/alpha-agp.c
index dd84af4..aad2756 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();
 	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..1dd1914 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();
 	if (!fake_bridge_dev) {
 		error = -ENOMEM;
 		goto fail;
diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 18798a7..4a15274 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -77,7 +77,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
 	struct resource *res;
 	struct pci_sriov *iov = dev->sriov;
 
-	virtfn = alloc_pci_dev();
+	virtfn = pci_alloc_dev();
 	if (!virtfn)
 		return -ENOMEM;
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 44a6f7d..44f29d3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
 	kfree(bridge);
 }
 
-struct pci_dev *alloc_pci_dev(void)
+struct pci_dev *pci_alloc_dev(void)
 {
 	struct pci_dev *dev;
 
@@ -1211,7 +1211,7 @@ struct pci_dev *alloc_pci_dev(void)
 
 	return dev;
 }
-EXPORT_SYMBOL(alloc_pci_dev);
+EXPORT_SYMBOL(pci_alloc_dev);
 
 bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
 				 int crs_timeout)
@@ -1261,7 +1261,7 @@ 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();
 	if (!dev)
 		return NULL;
 
diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 9504ec0..de2e61f 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();
 
 	if( *pdev == NULL ) return -1;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index c4d064d..67edd5a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -348,7 +348,7 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
 	return dev;
 }
 
-extern struct pci_dev *alloc_pci_dev(void);
+extern struct pci_dev *pci_alloc_dev(void);
 
 #define	to_pci_dev(n) container_of(n, struct pci_dev, dev)
 #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
-- 
1.7.1

Re: [PATCH 2/3] PCI: rename alloc_pci_dev() to pci_alloc_dev()

[Bjorn Helgaas]

On Thu, Apr 18, 2013 at 3:00 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> From a3135f4a814ec2faba007f030ca96ea2297484ef Mon Sep 17 00:00:00 2001
> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
> Date: Thu, 18 Apr 2013 17:27:40 +0900
> Subject: [PATCH 2/3] PCI: rename alloc_pci_dev() to pci_alloc_dev()
>
> Rename alloc_pci_dev() to pci_alloc_dev() to make the name style the same
> as pci_alloc_bus(), pci_destroy_dev().
>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> Reviewed-by: Li Guang <lig.fnst@cn.fujitsu.com>
> ---
>  arch/powerpc/kernel/pci_of_scan.c |    2 +-
>  drivers/char/agp/alpha-agp.c      |    2 +-
>  drivers/char/agp/parisc-agp.c     |    2 +-
>  drivers/pci/iov.c                 |    2 +-
>  drivers/pci/probe.c               |    6 +++---
>  drivers/scsi/megaraid.c           |    2 +-
>  include/linux/pci.h               |    2 +-
>  7 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
> index 9e0f6fd..2e35d11 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();
>         if (!dev)
>                 return NULL;
>         type = of_get_property(node, "device_type", NULL);
> diff --git a/drivers/char/agp/alpha-agp.c b/drivers/char/agp/alpha-agp.c
> index dd84af4..aad2756 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();
>         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..1dd1914 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();
>         if (!fake_bridge_dev) {
>                 error = -ENOMEM;
>                 goto fail;
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 18798a7..4a15274 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -77,7 +77,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
>         struct resource *res;
>         struct pci_sriov *iov = dev->sriov;
>
> -       virtfn = alloc_pci_dev();
> +       virtfn = pci_alloc_dev();
>         if (!virtfn)
>                 return -ENOMEM;
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 44a6f7d..44f29d3 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
>         kfree(bridge);
>  }
>
> -struct pci_dev *alloc_pci_dev(void)
> +struct pci_dev *pci_alloc_dev(void)
>  {
>         struct pci_dev *dev;
>
> @@ -1211,7 +1211,7 @@ struct pci_dev *alloc_pci_dev(void)
>
>         return dev;
>  }
> -EXPORT_SYMBOL(alloc_pci_dev);
> +EXPORT_SYMBOL(pci_alloc_dev);

I like these patches a lot.  But I think we have some logistical
issues to work out.

Since alloc_pci_dev() is exported, there may be out-of-tree modules
that use it, so I don't want to simply remove alloc_pci_dev()
completely.  I think it would be better to keep it for a while but
mark it as __deprecated.

You can introduce a new pci_alloc_dev() here, and since it's a new
interface, I think you might as well make it take a "struct pci_bus *"
and acquire the reference from the beginning.

I think you could then make the final patch be the only one that
touches powerpc, agp, megaraid, etc., to convert them from
alloc_pci_dev(void) to pci_alloc_dev(bus).

Bjorn

>  bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
>                                  int crs_timeout)
> @@ -1261,7 +1261,7 @@ 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();
>         if (!dev)
>                 return NULL;
>
> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
> index 9504ec0..de2e61f 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();
>
>         if( *pdev == NULL ) return -1;
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index c4d064d..67edd5a 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -348,7 +348,7 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
>         return dev;
>  }
>
> -extern struct pci_dev *alloc_pci_dev(void);
> +extern struct pci_dev *pci_alloc_dev(void);
>
>  #define        to_pci_dev(n) container_of(n, struct pci_dev, dev)
>  #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
> --
> 1.7.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] PCI: rename alloc_pci_dev() to pci_alloc_dev()

[Gu Zheng]

On 04/19/2013 12:00 AM, Bjorn Helgaas wrote:

> On Thu, Apr 18, 2013 at 3:00 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> From a3135f4a814ec2faba007f030ca96ea2297484ef Mon Sep 17 00:00:00 2001
>> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> Date: Thu, 18 Apr 2013 17:27:40 +0900
>> Subject: [PATCH 2/3] PCI: rename alloc_pci_dev() to pci_alloc_dev()
>>
>> Rename alloc_pci_dev() to pci_alloc_dev() to make the name style the same
>> as pci_alloc_bus(), pci_destroy_dev().
>>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> Reviewed-by: Li Guang <lig.fnst@cn.fujitsu.com>
>> ---
>>  arch/powerpc/kernel/pci_of_scan.c |    2 +-
>>  drivers/char/agp/alpha-agp.c      |    2 +-
>>  drivers/char/agp/parisc-agp.c     |    2 +-
>>  drivers/pci/iov.c                 |    2 +-
>>  drivers/pci/probe.c               |    6 +++---
>>  drivers/scsi/megaraid.c           |    2 +-
>>  include/linux/pci.h               |    2 +-
>>  7 files changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
>> index 9e0f6fd..2e35d11 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();
>>         if (!dev)
>>                 return NULL;
>>         type = of_get_property(node, "device_type", NULL);
>> diff --git a/drivers/char/agp/alpha-agp.c b/drivers/char/agp/alpha-agp.c
>> index dd84af4..aad2756 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();
>>         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..1dd1914 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();
>>         if (!fake_bridge_dev) {
>>                 error = -ENOMEM;
>>                 goto fail;
>> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>> index 18798a7..4a15274 100644
>> --- a/drivers/pci/iov.c
>> +++ b/drivers/pci/iov.c
>> @@ -77,7 +77,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
>>         struct resource *res;
>>         struct pci_sriov *iov = dev->sriov;
>>
>> -       virtfn = alloc_pci_dev();
>> +       virtfn = pci_alloc_dev();
>>         if (!virtfn)
>>                 return -ENOMEM;
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 44a6f7d..44f29d3 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
>>         kfree(bridge);
>>  }
>>
>> -struct pci_dev *alloc_pci_dev(void)
>> +struct pci_dev *pci_alloc_dev(void)
>>  {
>>         struct pci_dev *dev;
>>
>> @@ -1211,7 +1211,7 @@ struct pci_dev *alloc_pci_dev(void)
>>
>>         return dev;
>>  }
>> -EXPORT_SYMBOL(alloc_pci_dev);
>> +EXPORT_SYMBOL(pci_alloc_dev);
> 
> I like these patches a lot.  But I think we have some logistical
> issues to work out.
> 
> Since alloc_pci_dev() is exported, there may be out-of-tree modules
> that use it, so I don't want to simply remove alloc_pci_dev()
> completely.  I think it would be better to keep it for a while but
> mark it as __deprecated.

> 
> You can introduce a new pci_alloc_dev() here, and since it's a new
> interface, I think you might as well make it take a "struct pci_bus *"
> and acquire the reference from the beginning.
> 
> I think you could then make the final patch be the only one that
> touches powerpc, agp, megaraid, etc., to convert them from
> alloc_pci_dev(void) to pci_alloc_dev(bus).

Hi Bjorn,
	Thanks for your suggestions very much, and I'll send out a new version soon.

Best regards,
Gu 

> 
> Bjorn
> 
>>  bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
>>                                  int crs_timeout)
>> @@ -1261,7 +1261,7 @@ 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();
>>         if (!dev)
>>                 return NULL;
>>
>> diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
>> index 9504ec0..de2e61f 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();
>>
>>         if( *pdev == NULL ) return -1;
>>
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index c4d064d..67edd5a 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -348,7 +348,7 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
>>         return dev;
>>  }
>>
>> -extern struct pci_dev *alloc_pci_dev(void);
>> +extern struct pci_dev *pci_alloc_dev(void);
>>
>>  #define        to_pci_dev(n) container_of(n, struct pci_dev, dev)
>>  #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
>> --
>> 1.7.1
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH 1/2] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

[Gu Zheng]

>From 906167d9a09babbe189f62944ecb8c0b198a0f64 Mon Sep 17 00:00:00 2001
From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Fri, 19 Apr 2013 18:12:32 +0900
Subject: [PATCH 1/2] PCI: Introduce  pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

Now here we introduce a new struct pci_dev *pci_alloc_dev(struct pci_bus *bus) to replace alloc_pci_dev().
It take a "struct pci_bus *" argument, so we can alloc a pci device on a target pci bus, and it acquire
the reference of the pci_bus.
Since the old alloc_pci_dev() is exported, so we still keep it for a while but mark it as __deprecated.  

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 drivers/pci/probe.c |   21 ++++++++++++++++++++-
 include/linux/pci.h |    4 +++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b494066..5233fb6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
 	kfree(bridge);
 }
 
-struct pci_dev *alloc_pci_dev(void)
+struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
 
@@ -1209,6 +1209,25 @@ struct pci_dev *alloc_pci_dev(void)
 
 	INIT_LIST_HEAD(&dev->bus_list);
 
+	if (bus) {
+		get_device(&bus->dev);
+		dev->bus = bus;
+	}
+
+	return dev;
+}
+EXPORT_SYMBOL(pci_alloc_dev);
+
+__deprecated struct pci_dev *alloc_pci_dev(void)
+{
+	struct pci_dev *dev;
+	printk(KERN_DEBUG "alloc_pci_dev is deprecated, please use pci_alloc_dev(struct pci_bus *) instead!\n");
+	dev = kzalloc(sizeof(struct pci_dev), GFP_KERNEL);
+	if (!dev)
+		return NULL;
+
+	INIT_LIST_HEAD(&dev->bus_list);
+
 	return dev;
 }
 EXPORT_SYMBOL(alloc_pci_dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 710067f..682de2b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -348,7 +348,9 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
 	return dev;
 }
 
-extern struct pci_dev *alloc_pci_dev(void);
+extern struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
+
+extern __deprecated struct pci_dev *alloc_pci_dev(void);
 
 #define	to_pci_dev(n) container_of(n, struct pci_dev, dev)
 #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
-- 
1.7.1

Re: [PATCH 1/2] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

[Bjorn Helgaas]

On Fri, Apr 19, 2013 at 3:44 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> From 906167d9a09babbe189f62944ecb8c0b198a0f64 Mon Sep 17 00:00:00 2001
> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
> Date: Fri, 19 Apr 2013 18:12:32 +0900
> Subject: [PATCH 1/2] PCI: Introduce  pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()
>
> Now here we introduce a new struct pci_dev *pci_alloc_dev(struct pci_bus *bus) to replace alloc_pci_dev().
> It take a "struct pci_bus *" argument, so we can alloc a pci device on a target pci bus, and it acquire
> the reference of the pci_bus.
> Since the old alloc_pci_dev() is exported, so we still keep it for a while but mark it as __deprecated.
>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
>  drivers/pci/probe.c |   21 ++++++++++++++++++++-
>  include/linux/pci.h |    4 +++-
>  2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index b494066..5233fb6 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
>         kfree(bridge);
>  }
>
> -struct pci_dev *alloc_pci_dev(void)
> +struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
>  {
>         struct pci_dev *dev;
>
> @@ -1209,6 +1209,25 @@ struct pci_dev *alloc_pci_dev(void)
>
>         INIT_LIST_HEAD(&dev->bus_list);
>
> +       if (bus) {
> +               get_device(&bus->dev);
> +               dev->bus = bus;
> +       }
> +
> +       return dev;
> +}
> +EXPORT_SYMBOL(pci_alloc_dev);
> +
> +__deprecated struct pci_dev *alloc_pci_dev(void)

I don't think there's any point in marking the function *definition*
as deprecated; it only makes sense for the declaration in the header
file, so callers of the function will generate warnings.

> +{
> +       struct pci_dev *dev;
> +       printk(KERN_DEBUG "alloc_pci_dev is deprecated, please use pci_alloc_dev(struct pci_bus *) instead!\n");

I don't want to print a message at run-time, because users will see
the message and complain about it, but they can't do anything about
it.  And neither can we, because it will only be out-of-tree modules
that call alloc_pci_dev().  The build-time warning is all we can do.

> +       dev = kzalloc(sizeof(struct pci_dev), GFP_KERNEL);
> +       if (!dev)
> +               return NULL;
> +
> +       INIT_LIST_HEAD(&dev->bus_list);

Can't you implement this as simply:

    return pci_alloc_dev(NULL);

> +
>         return dev;
>  }
>  EXPORT_SYMBOL(alloc_pci_dev);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 710067f..682de2b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -348,7 +348,9 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
>         return dev;
>  }
>
> -extern struct pci_dev *alloc_pci_dev(void);
> +extern struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
> +
> +extern __deprecated struct pci_dev *alloc_pci_dev(void);
>
>  #define        to_pci_dev(n) container_of(n, struct pci_dev, dev)
>  #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
> --
> 1.7.1
>

Re: [PATCH 1/2] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

[Mike Qiu]

On Fri, 2013-04-19 at 11:32 -0600, Bjorn Helgaas wrote:
> On Fri, Apr 19, 2013 at 3:44 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> > From 906167d9a09babbe189f62944ecb8c0b198a0f64 Mon Sep 17 00:00:00 2001
> > From: Gu Zheng <guz.fnst@cn.fujitsu.com>
> > Date: Fri, 19 Apr 2013 18:12:32 +0900
> > Subject: [PATCH 1/2] PCI: Introduce  pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()
> >
> > Now here we introduce a new struct pci_dev *pci_alloc_dev(struct pci_bus *bus) to replace alloc_pci_dev().
> > It take a "struct pci_bus *" argument, so we can alloc a pci device on a target pci bus, and it acquire
> > the reference of the pci_bus.
> > Since the old alloc_pci_dev() is exported, so we still keep it for a while but mark it as __deprecated.
> >
> > Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> > ---
> >  drivers/pci/probe.c |   21 ++++++++++++++++++++-
> >  include/linux/pci.h |    4 +++-
> >  2 files changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index b494066..5233fb6 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
> >         kfree(bridge);
> >  }
> >
> > -struct pci_dev *alloc_pci_dev(void)
> > +struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
> >  {
> >         struct pci_dev *dev;
> >
> > @@ -1209,6 +1209,25 @@ struct pci_dev *alloc_pci_dev(void)
> >
> >         INIT_LIST_HEAD(&dev->bus_list);
> >
> > +       if (bus) {
> > +               get_device(&bus->dev);
> > +               dev->bus = bus;
> > +       }
> > +
> > +       return dev;
> > +}
> > +EXPORT_SYMBOL(pci_alloc_dev);
> > +
> > +__deprecated struct pci_dev *alloc_pci_dev(void)
> 
> I don't think there's any point in marking the function *definition*
> as deprecated; it only makes sense for the declaration in the header
> file, so callers of the function will generate warnings.
> 
> > +{
> > +       struct pci_dev *dev;
> > +       printk(KERN_DEBUG "alloc_pci_dev is deprecated, please use pci_alloc_dev(struct pci_bus *) instead!\n");
> 
> I don't want to print a message at run-time, because users will see
> the message and complain about it, but they can't do anything about
> it.  And neither can we, because it will only be out-of-tree modules
> that call alloc_pci_dev().  The build-time warning is all we can do.
Yes, totally agree .
> 
> > +       dev = kzalloc(sizeof(struct pci_dev), GFP_KERNEL);
> > +       if (!dev)
> > +               return NULL;
> > +
> > +       INIT_LIST_HEAD(&dev->bus_list);
> 
> Can't you implement this as simply:
> 
>     return pci_alloc_dev(NULL);
Yes, I think it's better to keep the old API, and modify it to call the
new one, this makes the modules which call the old fashion API feel
smoothly. and the new modules can use the new API.
> 
> > +
> >         return dev;
> >  }
> >  EXPORT_SYMBOL(alloc_pci_dev);
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 710067f..682de2b 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -348,7 +348,9 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
> >         return dev;
> >  }
> >
> > -extern struct pci_dev *alloc_pci_dev(void);
> > +extern struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
> > +
> > +extern __deprecated struct pci_dev *alloc_pci_dev(void);
> >
> >  #define        to_pci_dev(n) container_of(n, struct pci_dev, dev)
> >  #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
> > --
> > 1.7.1
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/2] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

[Gu Zheng]

On 04/20/2013 10:58 AM, Mike Qiu wrote:

> On Fri, 2013-04-19 at 11:32 -0600, Bjorn Helgaas wrote:
>> On Fri, Apr 19, 2013 at 3:44 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>>> From 906167d9a09babbe189f62944ecb8c0b198a0f64 Mon Sep 17 00:00:00 2001
>>> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
>>> Date: Fri, 19 Apr 2013 18:12:32 +0900
>>> Subject: [PATCH 1/2] PCI: Introduce  pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()
>>>
>>> Now here we introduce a new struct pci_dev *pci_alloc_dev(struct pci_bus *bus) to replace alloc_pci_dev().
>>> It take a "struct pci_bus *" argument, so we can alloc a pci device on a target pci bus, and it acquire
>>> the reference of the pci_bus.
>>> Since the old alloc_pci_dev() is exported, so we still keep it for a while but mark it as __deprecated.
>>>
>>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>>> ---
>>>  drivers/pci/probe.c |   21 ++++++++++++++++++++-
>>>  include/linux/pci.h |    4 +++-
>>>  2 files changed, 23 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>> index b494066..5233fb6 100644
>>> --- a/drivers/pci/probe.c
>>> +++ b/drivers/pci/probe.c
>>> @@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
>>>         kfree(bridge);
>>>  }
>>>
>>> -struct pci_dev *alloc_pci_dev(void)
>>> +struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
>>>  {
>>>         struct pci_dev *dev;
>>>
>>> @@ -1209,6 +1209,25 @@ struct pci_dev *alloc_pci_dev(void)
>>>
>>>         INIT_LIST_HEAD(&dev->bus_list);
>>>
>>> +       if (bus) {
>>> +               get_device(&bus->dev);
>>> +               dev->bus = bus;
>>> +       }
>>> +
>>> +       return dev;
>>> +}
>>> +EXPORT_SYMBOL(pci_alloc_dev);
>>> +
>>> +__deprecated struct pci_dev *alloc_pci_dev(void)
>>
>> I don't think there's any point in marking the function *definition*
>> as deprecated; it only makes sense for the declaration in the header
>> file, so callers of the function will generate warnings.
>>
>>> +{
>>> +       struct pci_dev *dev;
>>> +       printk(KERN_DEBUG "alloc_pci_dev is deprecated, please use pci_alloc_dev(struct pci_bus *) instead!\n");
>>
>> I don't want to print a message at run-time, because users will see
>> the message and complain about it, but they can't do anything about
>> it.  And neither can we, because it will only be out-of-tree modules
>> that call alloc_pci_dev().  The build-time warning is all we can do.
> Yes, totally agree .
>>
>>> +       dev = kzalloc(sizeof(struct pci_dev), GFP_KERNEL);
>>> +       if (!dev)
>>> +               return NULL;
>>> +
>>> +       INIT_LIST_HEAD(&dev->bus_list);
>>
>> Can't you implement this as simply:
>>
>>     return pci_alloc_dev(NULL);
> Yes, I think it's better to keep the old API, and modify it to call the
> new one, this makes the modules which call the old fashion API feel
> smoothly. and the new modules can use the new API.

Hi Mike,
    You are right! Thanks for your suggestion!

Regards,
Gu

>>
>>> +
>>>         return dev;
>>>  }
>>>  EXPORT_SYMBOL(alloc_pci_dev);
>>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>>> index 710067f..682de2b 100644
>>> --- a/include/linux/pci.h
>>> +++ b/include/linux/pci.h
>>> @@ -348,7 +348,9 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
>>>         return dev;
>>>  }
>>>
>>> -extern struct pci_dev *alloc_pci_dev(void);
>>> +extern struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
>>> +
>>> +extern __deprecated struct pci_dev *alloc_pci_dev(void);
>>>
>>>  #define        to_pci_dev(n) container_of(n, struct pci_dev, dev)
>>>  #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
>>> --
>>> 1.7.1
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 
> 

Re: [PATCH 1/2] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

[Gu Zheng]

Hi Bjorn,
   Sorry for the later reply!

On 04/20/2013 01:32 AM, Bjorn Helgaas wrote:

> On Fri, Apr 19, 2013 at 3:44 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> From 906167d9a09babbe189f62944ecb8c0b198a0f64 Mon Sep 17 00:00:00 2001
>> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> Date: Fri, 19 Apr 2013 18:12:32 +0900
>> Subject: [PATCH 1/2] PCI: Introduce  pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()
>>
>> Now here we introduce a new struct pci_dev *pci_alloc_dev(struct pci_bus *bus) to replace alloc_pci_dev().
>> It take a "struct pci_bus *" argument, so we can alloc a pci device on a target pci bus, and it acquire
>> the reference of the pci_bus.
>> Since the old alloc_pci_dev() is exported, so we still keep it for a while but mark it as __deprecated.
>>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> ---
>>  drivers/pci/probe.c |   21 ++++++++++++++++++++-
>>  include/linux/pci.h |    4 +++-
>>  2 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index b494066..5233fb6 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
>>         kfree(bridge);
>>  }
>>
>> -struct pci_dev *alloc_pci_dev(void)
>> +struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
>>  {
>>         struct pci_dev *dev;
>>
>> @@ -1209,6 +1209,25 @@ struct pci_dev *alloc_pci_dev(void)
>>
>>         INIT_LIST_HEAD(&dev->bus_list);
>>
>> +       if (bus) {
>> +               get_device(&bus->dev);
>> +               dev->bus = bus;
>> +       }
>> +
>> +       return dev;
>> +}
>> +EXPORT_SYMBOL(pci_alloc_dev);
>> +
>> +__deprecated struct pci_dev *alloc_pci_dev(void)
> 
> I don't think there's any point in marking the function *definition*
> as deprecated; it only makes sense for the declaration in the header
> file, so callers of the function will generate warnings.

Thanks for your explaining, I'll fix it in the right way.

> 
>> +{
>> +       struct pci_dev *dev;
>> +       printk(KERN_DEBUG "alloc_pci_dev is deprecated, please use pci_alloc_dev(struct pci_bus *) instead!\n");
> 
> I don't want to print a message at run-time, because users will see
> the message and complain about it, but they can't do anything about
> it.  And neither can we, because it will only be out-of-tree modules
> that call alloc_pci_dev().  The build-time warning is all we can do.

Yeah, it seems useless here. I'll remove it. 
 

> 
>> +       dev = kzalloc(sizeof(struct pci_dev), GFP_KERNEL);
>> +       if (!dev)
>> +               return NULL;
>> +
>> +       INIT_LIST_HEAD(&dev->bus_list);
> 
> Can't you implement this as simply:
> 
>     return pci_alloc_dev(NULL);

Agree, it can simplify the code, and make the later removal of the old alloc_pci_dev() smoothly.

> 
>> +
>>         return dev;
>>  }
>>  EXPORT_SYMBOL(alloc_pci_dev);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 710067f..682de2b 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -348,7 +348,9 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
>>         return dev;
>>  }
>>
>> -extern struct pci_dev *alloc_pci_dev(void);
>> +extern struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
>> +
>> +extern __deprecated struct pci_dev *alloc_pci_dev(void);
>>
>>  #define        to_pci_dev(n) container_of(n, struct pci_dev, dev)
>>  #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
>> --
>> 1.7.1
>>
> 

[PATCH 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Gu Zheng]

>From 2535fb1a2502b3d9bebc31f7c7205f42392ea5f9 Mon Sep 17 00:00:00 2001
From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Fri, 19 Apr 2013 17:46:36 +0900
Subject: [PATCH 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void).

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 arch/powerpc/kernel/pci_of_scan.c |    3 +--
 drivers/char/agp/alpha-agp.c      |    2 +-
 drivers/char/agp/parisc-agp.c     |    2 +-
 drivers/pci/iov.c                 |    2 +-
 drivers/pci/probe.c               |    3 +--
 drivers/scsi/megaraid.c           |    2 +-
 6 files changed, 6 insertions(+), 8 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/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..dba2031 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -76,7 +76,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
 	struct resource *res;
 	struct pci_sriov *iov = dev->sriov;
 
-	virtfn = alloc_pci_dev();
+	virtfn = pci_alloc_dev(NULL);
 	if (!virtfn)
 		return -ENOMEM;
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 5233fb6..59637a6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1280,11 +1280,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.7.1

Re: [PATCH 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Bjorn Helgaas]

On Fri, Apr 19, 2013 at 3:45 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> From 2535fb1a2502b3d9bebc31f7c7205f42392ea5f9 Mon Sep 17 00:00:00 2001
> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
> Date: Fri, 19 Apr 2013 17:46:36 +0900
> Subject: [PATCH 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead
>
> Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void).

You didn't do anything with Yinghai's comment.  He's 100% correct: you
have to add the put_device() that corresponds to the new get_device()
calls you're adding.  Both have to be in the same patch.

> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> ---
>  arch/powerpc/kernel/pci_of_scan.c |    3 +--
>  drivers/char/agp/alpha-agp.c      |    2 +-
>  drivers/char/agp/parisc-agp.c     |    2 +-
>  drivers/pci/iov.c                 |    2 +-
>  drivers/pci/probe.c               |    3 +--
>  drivers/scsi/megaraid.c           |    2 +-
>  6 files changed, 6 insertions(+), 8 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/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..dba2031 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -76,7 +76,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
>         struct resource *res;
>         struct pci_sriov *iov = dev->sriov;
>
> -       virtfn = alloc_pci_dev();
> +       virtfn = pci_alloc_dev(NULL);
>         if (!virtfn)
>                 return -ENOMEM;
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 5233fb6..59637a6 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1280,11 +1280,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.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Gu Zheng]

On 04/20/2013 01:35 AM, Bjorn Helgaas wrote:

> On Fri, Apr 19, 2013 at 3:45 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> From 2535fb1a2502b3d9bebc31f7c7205f42392ea5f9 Mon Sep 17 00:00:00 2001
>> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> Date: Fri, 19 Apr 2013 17:46:36 +0900
>> Subject: [PATCH 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead
>>
>> Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void).
> 
> You didn't do anything with Yinghai's comment.  He's 100% correct: you
> have to add the put_device() that corresponds to the new get_device()
> calls you're adding.  Both have to be in the same patch.

Hi Bjorn,
	Thanks for your reminder, Yinghai's comment is completely right, the get_device()/put_device() should come in pairs.
I'll add the put_device() in the next version.

Best regards,
Gu

> 
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> ---
>>  arch/powerpc/kernel/pci_of_scan.c |    3 +--
>>  drivers/char/agp/alpha-agp.c      |    2 +-
>>  drivers/char/agp/parisc-agp.c     |    2 +-
>>  drivers/pci/iov.c                 |    2 +-
>>  drivers/pci/probe.c               |    3 +--
>>  drivers/scsi/megaraid.c           |    2 +-
>>  6 files changed, 6 insertions(+), 8 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/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..dba2031 100644
>> --- a/drivers/pci/iov.c
>> +++ b/drivers/pci/iov.c
>> @@ -76,7 +76,7 @@ static int virtfn_add(struct pci_dev *dev, int id, int reset)
>>         struct resource *res;
>>         struct pci_sriov *iov = dev->sriov;
>>
>> -       virtfn = alloc_pci_dev();
>> +       virtfn = pci_alloc_dev(NULL);
>>         if (!virtfn)
>>                 return -ENOMEM;
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 5233fb6..59637a6 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1280,11 +1280,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.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v2 1/2] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

[Gu Zheng]

>From d66f5b255abeedfbeff212b1e4bc3d8f4ae908e3 Mon Sep 17 00:00:00 2001
From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Mon, 22 Apr 2013 19:04:07 +0900
Subject: [PATCH v2 1/2] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

Now here we introduce a new struct pci_dev *pci_alloc_dev(struct pci_bus *bus) to replace alloc_pci_dev().
It take a "struct pci_bus *" argument, so we can alloc a pci device on a target pci bus, and it acquire
the reference of the pci_bus.
Since the old alloc_pci_dev() is exported, so we still keep it for a while but mark it as __deprecated.

v2:
  Updated per code reviews from Bjorn Helgaas and Mike Qiu.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 drivers/pci/probe.c |   13 ++++++++++++-
 include/linux/pci.h |    4 +++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b494066..0bb92e4 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1199,7 +1199,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
 	kfree(bridge);
 }
 
-struct pci_dev *alloc_pci_dev(void)
+struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
 
@@ -1209,8 +1209,19 @@ struct pci_dev *alloc_pci_dev(void)
 
 	INIT_LIST_HEAD(&dev->bus_list);
 
+	if (bus) {
+		get_device(&bus->dev);
+		dev->bus = bus;
+	}
+
 	return dev;
 }
+EXPORT_SYMBOL(pci_alloc_dev);
+
+struct pci_dev *alloc_pci_dev(void)
+{
+	return pci_alloc_dev(NULL);
+}
 EXPORT_SYMBOL(alloc_pci_dev);
 
 bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 710067f..682de2b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -348,7 +348,9 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
 	return dev;
 }
 
-extern struct pci_dev *alloc_pci_dev(void);
+extern struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
+
+extern __deprecated struct pci_dev *alloc_pci_dev(void);
 
 #define	to_pci_dev(n) container_of(n, struct pci_dev, dev)
 #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
-- 
1.7.7

[PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Gu Zheng]

>From b01e61759f894b651ed3bb39c9fc18e0b66d7cb3 Mon Sep 17 00:00:00 2001
From: GuZheng <guz.fnst@cn.fujitsu.com>
Date: Tue, 23 Apr 2013 14:01:39 +0800
Subject: [PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus)
 instead

Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void).
And new functions pci_bus_get/put() are introdued to hide pci_bus' reference management.

v2:
  Follow Yinghai's suggestions to reduce pci_bus' ref when destroying pci device.
  Add new functions pci_bus_get/put() to hide pci_bus' reference management.

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/bus.c                 |   15 +++++++++++++++
 drivers/pci/iov.c                 |    8 +++++---
 drivers/pci/probe.c               |    9 +++------
 drivers/pci/remove.c              |    2 ++
 drivers/scsi/megaraid.c           |    2 +-
 include/linux/pci.h               |    3 +++
 10 files changed, 33 insertions(+), 16 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/bus.c b/drivers/pci/bus.c
index 8647dc6..5eacf78 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -18,6 +18,21 @@
 
 #include "pci.h"
 
+struct pci_bus *pci_bus_get(struct pci_bus *bus)
+{
+	if (bus)
+		get_device(&bus->dev);
+	return bus;
+}
+EXPORT_SYMBOL(pci_bus_get);
+
+void pci_bus_put(struct pci_bus *bus)
+{
+	if (bus)
+		put_device(&bus->dev);
+}
+EXPORT_SYMBOL(pci_bus_put);
+
 void pci_add_resource_offset(struct list_head *resources, struct resource *res,
 			     resource_size_t offset)
 {
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 0bb92e4..9d37d46 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1209,10 +1209,8 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
 
 	INIT_LIST_HEAD(&dev->bus_list);
 
-	if (bus) {
-		get_device(&bus->dev);
-		dev->bus = bus;
-	}
+	if (bus)
+		dev->bus = pci_bus_get(bus);
 
 	return dev;
 }
@@ -1272,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/pci/remove.c b/drivers/pci/remove.c
index cc875e6..fffdee8 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -38,6 +38,8 @@ static void pci_destroy_dev(struct pci_dev *dev)
 	list_del(&dev->bus_list);
 	up_write(&pci_bus_sem);
 
+	pci_bus_put(dev->bus);
+
 	pci_free_resources(dev);
 	put_device(&dev->dev);
 }
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;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 682de2b..c5f245d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1002,6 +1002,9 @@ int pci_request_selected_regions_exclusive(struct pci_dev *, int, const char *);
 void pci_release_selected_regions(struct pci_dev *, int);
 
 /* drivers/pci/bus.c */
+extern struct pci_bus *pci_bus_get(struct pci_bus *bus);
+extern void pci_bus_put(struct pci_bus *bus);
+
 void pci_add_resource(struct list_head *resources, struct resource *res);
 void pci_add_resource_offset(struct list_head *resources, struct resource *res,
 			     resource_size_t offset);
-- 
1.7.7

Re: [PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Jiang Liu]

On 04/23/2013 03:29 PM, Gu Zheng wrote:
> From b01e61759f894b651ed3bb39c9fc18e0b66d7cb3 Mon Sep 17 00:00:00 2001
> From: GuZheng <guz.fnst@cn.fujitsu.com>
> Date: Tue, 23 Apr 2013 14:01:39 +0800
> Subject: [PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus)
>  instead
> 
> Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void).
> And new functions pci_bus_get/put() are introdued to hide pci_bus' reference management.
> 
> v2:
>   Follow Yinghai's suggestions to reduce pci_bus' ref when destroying pci device.
>   Add new functions pci_bus_get/put() to hide pci_bus' reference management.
> 
> 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/bus.c                 |   15 +++++++++++++++
>  drivers/pci/iov.c                 |    8 +++++---
>  drivers/pci/probe.c               |    9 +++------
>  drivers/pci/remove.c              |    2 ++
>  drivers/scsi/megaraid.c           |    2 +-
>  include/linux/pci.h               |    3 +++
>  10 files changed, 33 insertions(+), 16 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/bus.c b/drivers/pci/bus.c
> index 8647dc6..5eacf78 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -18,6 +18,21 @@
>  
>  #include "pci.h"
>  
> +struct pci_bus *pci_bus_get(struct pci_bus *bus)
> +{
> +	if (bus)
> +		get_device(&bus->dev);
> +	return bus;
> +}
> +EXPORT_SYMBOL(pci_bus_get);
> +
> +void pci_bus_put(struct pci_bus *bus)
> +{
> +	if (bus)
> +		put_device(&bus->dev);
> +}
> +EXPORT_SYMBOL(pci_bus_put);
> +
>  void pci_add_resource_offset(struct list_head *resources, struct resource *res,
>  			     resource_size_t offset)
>  {
Hi Zheng,
	Could you please help to split addition of pci_bus_get() and pci_bus_put()
into a separate patch? I'm working on some patches which depends on this.

> 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 0bb92e4..9d37d46 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1209,10 +1209,8 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
>  
>  	INIT_LIST_HEAD(&dev->bus_list);
>  
> -	if (bus) {
> -		get_device(&bus->dev);
> -		dev->bus = bus;
> -	}
> +	if (bus)
How about remove above redundant check?

> +		dev->bus = pci_bus_get(bus);
>  
>  	return dev;
>  }
> @@ -1272,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/pci/remove.c b/drivers/pci/remove.c
> index cc875e6..fffdee8 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -38,6 +38,8 @@ static void pci_destroy_dev(struct pci_dev *dev)
>  	list_del(&dev->bus_list);
>  	up_write(&pci_bus_sem);
>  
> +	pci_bus_put(dev->bus);
> +
>  	pci_free_resources(dev);
>  	put_device(&dev->dev);
>  }
> 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;
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 682de2b..c5f245d 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1002,6 +1002,9 @@ int pci_request_selected_regions_exclusive(struct pci_dev *, int, const char *);
>  void pci_release_selected_regions(struct pci_dev *, int);
>  
>  /* drivers/pci/bus.c */
> +extern struct pci_bus *pci_bus_get(struct pci_bus *bus);
> +extern void pci_bus_put(struct pci_bus *bus);
> +
>  void pci_add_resource(struct list_head *resources, struct resource *res);
>  void pci_add_resource_offset(struct list_head *resources, struct resource *res,
>  			     resource_size_t offset);
> 

Re: [PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Gu Zheng]

On 04/24/2013 12:44 AM, Jiang Liu wrote:

> On 04/23/2013 03:29 PM, Gu Zheng wrote:
>> From b01e61759f894b651ed3bb39c9fc18e0b66d7cb3 Mon Sep 17 00:00:00 2001
>> From: GuZheng <guz.fnst@cn.fujitsu.com>
>> Date: Tue, 23 Apr 2013 14:01:39 +0800
>> Subject: [PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus)
>>  instead
>>
>> Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void).
>> And new functions pci_bus_get/put() are introdued to hide pci_bus' reference management.
>>
>> v2:
>>   Follow Yinghai's suggestions to reduce pci_bus' ref when destroying pci device.
>>   Add new functions pci_bus_get/put() to hide pci_bus' reference management.
>>
>> 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/bus.c                 |   15 +++++++++++++++
>>  drivers/pci/iov.c                 |    8 +++++---
>>  drivers/pci/probe.c               |    9 +++------
>>  drivers/pci/remove.c              |    2 ++
>>  drivers/scsi/megaraid.c           |    2 +-
>>  include/linux/pci.h               |    3 +++
>>  10 files changed, 33 insertions(+), 16 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/bus.c b/drivers/pci/bus.c
>> index 8647dc6..5eacf78 100644
>> --- a/drivers/pci/bus.c
>> +++ b/drivers/pci/bus.c
>> @@ -18,6 +18,21 @@
>>  
>>  #include "pci.h"
>>  
>> +struct pci_bus *pci_bus_get(struct pci_bus *bus)
>> +{
>> +	if (bus)
>> +		get_device(&bus->dev);
>> +	return bus;
>> +}
>> +EXPORT_SYMBOL(pci_bus_get);
>> +
>> +void pci_bus_put(struct pci_bus *bus)
>> +{
>> +	if (bus)
>> +		put_device(&bus->dev);
>> +}
>> +EXPORT_SYMBOL(pci_bus_put);
>> +
>>  void pci_add_resource_offset(struct list_head *resources, struct resource *res,
>>  			     resource_size_t offset)
>>  {
> Hi Zheng,
> 	Could you please help to split addition of pci_bus_get() and pci_bus_put()
> into a separate patch? I'm working on some patches which depends on this.

Sure, a glad thing that it is also useful to you!:)

> 
>> 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 0bb92e4..9d37d46 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1209,10 +1209,8 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
>>  
>>  	INIT_LIST_HEAD(&dev->bus_list);
>>  
>> -	if (bus) {
>> -		get_device(&bus->dev);
>> -		dev->bus = bus;
>> -	}
>> +	if (bus)
> How about remove above redundant check?

Thanks for your review.
Yeah, the pre-check seems redundant, but I think it can avoid the meaningless calling
pci_bus_get() and setting dev->bus.

Best regards,
Gu

> 
>> +		dev->bus = pci_bus_get(bus);
>>  
>>  	return dev;
>>  }
>> @@ -1272,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/pci/remove.c b/drivers/pci/remove.c
>> index cc875e6..fffdee8 100644
>> --- a/drivers/pci/remove.c
>> +++ b/drivers/pci/remove.c
>> @@ -38,6 +38,8 @@ static void pci_destroy_dev(struct pci_dev *dev)
>>  	list_del(&dev->bus_list);
>>  	up_write(&pci_bus_sem);
>>  
>> +	pci_bus_put(dev->bus);
>> +
>>  	pci_free_resources(dev);
>>  	put_device(&dev->dev);
>>  }
>> 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;
>>  
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 682de2b..c5f245d 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1002,6 +1002,9 @@ int pci_request_selected_regions_exclusive(struct pci_dev *, int, const char *);
>>  void pci_release_selected_regions(struct pci_dev *, int);
>>  
>>  /* drivers/pci/bus.c */
>> +extern struct pci_bus *pci_bus_get(struct pci_bus *bus);
>> +extern void pci_bus_put(struct pci_bus *bus);
>> +
>>  void pci_add_resource(struct list_head *resources, struct resource *res);
>>  void pci_add_resource_offset(struct list_head *resources, struct resource *res,
>>  			     resource_size_t offset);
>>
> 
> 

Re: [PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Yinghai Lu]

On Tue, Apr 23, 2013 at 12:29 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> From b01e61759f894b651ed3bb39c9fc18e0b66d7cb3 Mon Sep 17 00:00:00 2001
> From: GuZheng <guz.fnst@cn.fujitsu.com>
> Date: Tue, 23 Apr 2013 14:01:39 +0800
> Subject: [PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus)
>  instead
>
> Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void).
> And new functions pci_bus_get/put() are introdued to hide pci_bus' reference management.
>
> v2:
>   Follow Yinghai's suggestions to reduce pci_bus' ref when destroying pci device.
>   Add new functions pci_bus_get/put() to hide pci_bus' reference management.

You did not tell us if the remove test scripts works after put the line back.

If it does not work, what is the point of this patch?

Yinghai

Re: [PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Gu Zheng]

On 04/24/2013 01:34 AM, Yinghai Lu wrote:

> On Tue, Apr 23, 2013 at 12:29 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> From b01e61759f894b651ed3bb39c9fc18e0b66d7cb3 Mon Sep 17 00:00:00 2001
>> From: GuZheng <guz.fnst@cn.fujitsu.com>
>> Date: Tue, 23 Apr 2013 14:01:39 +0800
>> Subject: [PATCH v2 2/2] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus)
>>  instead
>>
>> Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void).
>> And new functions pci_bus_get/put() are introdued to hide pci_bus' reference management.
>>
>> v2:
>>   Follow Yinghai's suggestions to reduce pci_bus' ref when destroying pci device.
>>   Add new functions pci_bus_get/put() to hide pci_bus' reference management.
> 
> You did not tell us if the remove test scripts works after put the line back.

Hi Yinghai,
    As you know, I split this patch from the original bug-fix one, and this one is
not used to fix the bug we discussed. My test box is on the duty of other tests, I'll do the
test and send out the result as soon as it is ready for me.

BTW, Could please split a while to answer the two questions I asked you yesterday?
> 

> If it does not work, what is the point of this patch?

I think the patch title and the change log can explain all. It introduce a new pci_alloc_dev(bus)
to replace the alloc_pci_dev(), and add the pci_bus' reference management when we deal with its downstream
pci devices.

Thanks,
Gu

> 
> Yinghai
> 

[PATCH v2 1/4] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()

[Gu Zheng]

>From 888e824c01f9c039622a2ab74c3df3b884fd7b12 Mon Sep 17 00:00:00 2001
From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Mon, 22 Apr 2013 19:04:07 +0900
Subject: [PATCH v2 1/4] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace
 alloc_pci_dev()

Now here we introduce a new struct pci_dev *pci_alloc_dev(struct pci_bus *bus) to replace alloc_pci_dev().
It take a "struct pci_bus *" argument, so we can alloc a pci device on a target pci bus, and it acquire
the reference of the pci_bus.
We use pci_alloc_dev(NULL) to simplify the old alloc_pci_dev(), and keep it for a while but mark it as __deprecated.

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 drivers/pci/probe.c |   13 ++++++++++++-
 include/linux/pci.h |    4 +++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 43ece5d..89db76a 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1201,7 +1201,7 @@ static void pci_release_bus_bridge_dev(struct device *dev)
 	kfree(bridge);
 }
 
-struct pci_dev *alloc_pci_dev(void)
+struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
 
@@ -1211,8 +1211,19 @@ struct pci_dev *alloc_pci_dev(void)
 
 	INIT_LIST_HEAD(&dev->bus_list);
 
+	if (bus) {
+		get_device(&bus->dev);
+		dev->bus = bus;
+	}
+
 	return dev;
 }
+EXPORT_SYMBOL(pci_alloc_dev);
+
+struct pci_dev *alloc_pci_dev(void)
+{
+	return pci_alloc_dev(NULL);
+}
 EXPORT_SYMBOL(alloc_pci_dev);
 
 bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e73dfa3..6c09949 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -364,7 +364,9 @@ static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
 	return dev;
 }
 
-struct pci_dev *alloc_pci_dev(void);
+struct pci_dev *pci_alloc_dev(struct pci_bus *bus);
+
+__deprecated struct pci_dev *alloc_pci_dev(void);
 
 #define	to_pci_dev(n) container_of(n, struct pci_dev, dev)
 #define for_each_pci_dev(d) while ((d = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, d)) != NULL)
-- 
1.7.7

[PATCH v2 2/4] PCI: introduce pci_bus_get()/pci_bus_put() to hide pci_bus' reference management

[Gu Zheng]

>From ff785e84afd2558dd52de8e73c4f75b7563a595b Mon Sep 17 00:00:00 2001
From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Tue, 30 Apr 2013 17:22:02 +0900
Subject: [PATCH v2 2/4] PCI: introduce pci_bus_get()/pci_bus_put() to hide
 pci_bus' reference management

Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
---
 drivers/pci/bus.c   |   15 +++++++++++++++
 include/linux/pci.h |    3 +++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 748f8f3..93f4dc0 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -18,6 +18,21 @@
 
 #include "pci.h"
 
+struct pci_bus *pci_bus_get(struct pci_bus *bus)
+{
+	if (bus)
+		get_device(&bus->dev);
+	return bus;
+}
+EXPORT_SYMBOL(pci_bus_get);
+
+void pci_bus_put(struct pci_bus *bus)
+{
+	if (bus)
+		put_device(&bus->dev);
+}
+EXPORT_SYMBOL(pci_bus_put);
+
 void pci_add_resource_offset(struct list_head *resources, struct resource *res,
 			     resource_size_t offset)
 {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 6c09949..2581e53 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1020,6 +1020,9 @@ int pci_request_selected_regions_exclusive(struct pci_dev *, int, const char *);
 void pci_release_selected_regions(struct pci_dev *, int);
 
 /* drivers/pci/bus.c */
+extern struct pci_bus *pci_bus_get(struct pci_bus *bus);
+extern void pci_bus_put(struct pci_bus *bus);
+
 void pci_add_resource(struct list_head *resources, struct resource *res);
 void pci_add_resource_offset(struct list_head *resources, struct resource *res,
 			     resource_size_t offset);
-- 
1.7.7

[PATCH v2 3/4] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead

[Gu Zheng]

>From 055d93b0e9a29f3786996f90b7893bd6eb4d244b Mon Sep 17 00:00:00 2001
From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Tue, 30 Apr 2013 17:23:31 +0900
Subject: [PATCH v2 3/4] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus)
 instead

Use the new pci_alloc_dev(bus) to replace the existing using of alloc_pci_dev(void).
And new functions pci_bus_get/put() are introdued to hide pci_bus' reference management.

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               |   10 ++++------
 drivers/scsi/megaraid.c           |    2 +-
 7 files changed, 14 insertions(+), 16 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 89db76a..32cf908 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);
@@ -1211,10 +1212,8 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
 
 	INIT_LIST_HEAD(&dev->bus_list);
 
-	if (bus) {
-		get_device(&bus->dev);
-		dev->bus = bus;
-	}
+	if (bus)
+		dev->bus = pci_bus_get(bus);
 
 	return dev;
 }
@@ -1274,11 +1273,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.7.7

[PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback()

[Gu Zheng]

>From a870da3615988f53a8949e5f8c907b079162067b Mon Sep 17 00:00:00 2001
From: Gu Zheng <guz.fnst@cn.fujitsu.com>
Date: Tue, 30 Apr 2013 18:45:12 +0800
Subject: [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback 
We found nested removing through:
	echo -n 1 > /sys/bus/pci/devices/0000\:10\:00.0/remove ; echo -n 1 >
/sys/bus/pci/devices/0000\:1a\:01.0/remove

will cause kernel crash as bus get freed.

[  418.946462] CPU 4
[  418.968377] Pid: 512, comm: kworker/u:2 Tainted: G        W    3.8.0 #2
FUJITSU-SV PRIMEQUEST 1800E/SB
[  419.081763] RIP: 0010:[<ffffffff8137972e>]  [<ffffffff8137972e>]
pci_bus_read_config_word+0x5e/0x90
[  420.494137] Call Trace:
[  420.523326]  [<ffffffff813851ef>] ? remove_callback+0x1f/0x40
[  420.591984]  [<ffffffff8138044b>] pci_pme_active+0x4b/0x1c0
[  420.658545]  [<ffffffff8137d8e7>] pci_stop_bus_device+0x57/0xb0
[  420.729259]  [<ffffffff8137dab6>] pci_stop_and_remove_bus_device+0x16/0x30
[  420.811392]  [<ffffffff813851fb>] remove_callback+0x2b/0x40
[  420.877955]  [<ffffffff81257a56>] sysfs_schedule_callback_work+0x26/0x70

https://bugzilla.kernel.org/show_bug.cgi?id=54411

We have one patch that will let device hold bus ref to prevent it from
being freed, but that will still generate warning.

------------[ cut here ]------------
WARNING: at lib/list_debug.c:53 __list_del_entry+0x63/0xd0()
Hardware name: PRIMEQUEST 1800E
list_del corruption, ffff8807d1b6c000->next is LIST_POISON1 (dead000000100100)
Call Trace:
 [<ffffffff81056d4f>] warn_slowpath_common+0x7f/0xc0
 [<ffffffff81056e46>] warn_slowpath_fmt+0x46/0x50
 [<ffffffff81280b13>] __list_del_entry+0x63/0xd0
 [<ffffffff81280b91>] list_del+0x11/0x40
 [<ffffffff81298331>] pci_destroy_dev+0x31/0xc0
 [<ffffffff812985bb>] pci_remove_bus_device+0x5b/0x70
 [<ffffffff812985ee>] pci_stop_and_remove_bus_device+0x1e/0x30
 [<ffffffff8129fc89>] remove_callback+0x29/0x40
 [<ffffffff811f3b84>] sysfs_schedule_callback_work+0x24/0x70

We can just check if the device get removed from pci tree
already in the protection under pci_remove_rescan_mutex.


Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/pci-sysfs.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 5b4a9d9..18590c1 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -328,10 +328,17 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
 
 static void remove_callback(struct device *dev)
 {
-	struct pci_dev *pdev = to_pci_dev(dev);
-
+	struct pci_dev *pdev = to_pci_dev(dev), *tmp;
+	bool found = false;
+	struct pci_bus *bus = pdev->bus;
 	mutex_lock(&pci_remove_rescan_mutex);
-	pci_stop_and_remove_bus_device(pdev);
+	list_for_each_entry(tmp, &bus->devices, bus_list)
+		if (tmp == pdev) {
+			found = true;
+			break;
+		}
+	if (found)
+		pci_stop_and_remove_bus_device(pdev);
 	mutex_unlock(&pci_remove_rescan_mutex);
 }
 
-- 
1.7.7

Re: [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback()

[Bjorn Helgaas]

On Tue, Apr 30, 2013 at 4:31 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
> From a870da3615988f53a8949e5f8c907b079162067b Mon Sep 17 00:00:00 2001
> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
> Date: Tue, 30 Apr 2013 18:45:12 +0800
> Subject: [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback
> We found nested removing through:
>         echo -n 1 > /sys/bus/pci/devices/0000\:10\:00.0/remove ; echo -n 1 >
> /sys/bus/pci/devices/0000\:1a\:01.0/remove
>
> will cause kernel crash as bus get freed.
>
> [  418.946462] CPU 4
> [  418.968377] Pid: 512, comm: kworker/u:2 Tainted: G        W    3.8.0 #2
> FUJITSU-SV PRIMEQUEST 1800E/SB
> [  419.081763] RIP: 0010:[<ffffffff8137972e>]  [<ffffffff8137972e>]
> pci_bus_read_config_word+0x5e/0x90
> [  420.494137] Call Trace:
> [  420.523326]  [<ffffffff813851ef>] ? remove_callback+0x1f/0x40
> [  420.591984]  [<ffffffff8138044b>] pci_pme_active+0x4b/0x1c0
> [  420.658545]  [<ffffffff8137d8e7>] pci_stop_bus_device+0x57/0xb0
> [  420.729259]  [<ffffffff8137dab6>] pci_stop_and_remove_bus_device+0x16/0x30
> [  420.811392]  [<ffffffff813851fb>] remove_callback+0x2b/0x40
> [  420.877955]  [<ffffffff81257a56>] sysfs_schedule_callback_work+0x26/0x70
>
> https://bugzilla.kernel.org/show_bug.cgi?id=54411
>
> We have one patch that will let device hold bus ref to prevent it from
> being freed, but that will still generate warning.
>
> ------------[ cut here ]------------
> WARNING: at lib/list_debug.c:53 __list_del_entry+0x63/0xd0()
> Hardware name: PRIMEQUEST 1800E
> list_del corruption, ffff8807d1b6c000->next is LIST_POISON1 (dead000000100100)
> Call Trace:
>  [<ffffffff81056d4f>] warn_slowpath_common+0x7f/0xc0
>  [<ffffffff81056e46>] warn_slowpath_fmt+0x46/0x50
>  [<ffffffff81280b13>] __list_del_entry+0x63/0xd0
>  [<ffffffff81280b91>] list_del+0x11/0x40
>  [<ffffffff81298331>] pci_destroy_dev+0x31/0xc0
>  [<ffffffff812985bb>] pci_remove_bus_device+0x5b/0x70
>  [<ffffffff812985ee>] pci_stop_and_remove_bus_device+0x1e/0x30
>  [<ffffffff8129fc89>] remove_callback+0x29/0x40
>  [<ffffffff811f3b84>] sysfs_schedule_callback_work+0x24/0x70
>
> We can just check if the device get removed from pci tree
> already in the protection under pci_remove_rescan_mutex.
>
>
> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/pci-sysfs.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 5b4a9d9..18590c1 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -328,10 +328,17 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
>
>  static void remove_callback(struct device *dev)
>  {
> -       struct pci_dev *pdev = to_pci_dev(dev);
> -
> +       struct pci_dev *pdev = to_pci_dev(dev), *tmp;
> +       bool found = false;
> +       struct pci_bus *bus = pdev->bus;
>         mutex_lock(&pci_remove_rescan_mutex);
> -       pci_stop_and_remove_bus_device(pdev);
> +       list_for_each_entry(tmp, &bus->devices, bus_list)
> +               if (tmp == pdev) {
> +                       found = true;
> +                       break;
> +               }
> +       if (found)
> +               pci_stop_and_remove_bus_device(pdev);

I assume this series won't be needed when we eventually get the
reference counting sorted out, because we shouldn't have to check
whether the device still exists.  Yinghai posted a
"fix_racing_removing_5.patch", which currently doesn't apply cleanly,
but I assume he'll refresh it after v3.10-rc1 is out.

Bjorn

>         mutex_unlock(&pci_remove_rescan_mutex);
>  }
>
> --
> 1.7.7
>

Re: [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback()

[Gu Zheng]

On 05/09/2013 06:32 AM, Bjorn Helgaas wrote:

> On Tue, Apr 30, 2013 at 4:31 AM, Gu Zheng <guz.fnst@cn.fujitsu.com> wrote:
>> From a870da3615988f53a8949e5f8c907b079162067b Mon Sep 17 00:00:00 2001
>> From: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> Date: Tue, 30 Apr 2013 18:45:12 +0800
>> Subject: [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback
>> We found nested removing through:
>>         echo -n 1 > /sys/bus/pci/devices/0000\:10\:00.0/remove ; echo -n 1 >
>> /sys/bus/pci/devices/0000\:1a\:01.0/remove
>>
>> will cause kernel crash as bus get freed.
>>
>> [  418.946462] CPU 4
>> [  418.968377] Pid: 512, comm: kworker/u:2 Tainted: G        W    3.8.0 #2
>> FUJITSU-SV PRIMEQUEST 1800E/SB
>> [  419.081763] RIP: 0010:[<ffffffff8137972e>]  [<ffffffff8137972e>]
>> pci_bus_read_config_word+0x5e/0x90
>> [  420.494137] Call Trace:
>> [  420.523326]  [<ffffffff813851ef>] ? remove_callback+0x1f/0x40
>> [  420.591984]  [<ffffffff8138044b>] pci_pme_active+0x4b/0x1c0
>> [  420.658545]  [<ffffffff8137d8e7>] pci_stop_bus_device+0x57/0xb0
>> [  420.729259]  [<ffffffff8137dab6>] pci_stop_and_remove_bus_device+0x16/0x30
>> [  420.811392]  [<ffffffff813851fb>] remove_callback+0x2b/0x40
>> [  420.877955]  [<ffffffff81257a56>] sysfs_schedule_callback_work+0x26/0x70
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=54411
>>
>> We have one patch that will let device hold bus ref to prevent it from
>> being freed, but that will still generate warning.
>>
>> ------------[ cut here ]------------
>> WARNING: at lib/list_debug.c:53 __list_del_entry+0x63/0xd0()
>> Hardware name: PRIMEQUEST 1800E
>> list_del corruption, ffff8807d1b6c000->next is LIST_POISON1 (dead000000100100)
>> Call Trace:
>>  [<ffffffff81056d4f>] warn_slowpath_common+0x7f/0xc0
>>  [<ffffffff81056e46>] warn_slowpath_fmt+0x46/0x50
>>  [<ffffffff81280b13>] __list_del_entry+0x63/0xd0
>>  [<ffffffff81280b91>] list_del+0x11/0x40
>>  [<ffffffff81298331>] pci_destroy_dev+0x31/0xc0
>>  [<ffffffff812985bb>] pci_remove_bus_device+0x5b/0x70
>>  [<ffffffff812985ee>] pci_stop_and_remove_bus_device+0x1e/0x30
>>  [<ffffffff8129fc89>] remove_callback+0x29/0x40
>>  [<ffffffff811f3b84>] sysfs_schedule_callback_work+0x24/0x70
>>
>> We can just check if the device get removed from pci tree
>> already in the protection under pci_remove_rescan_mutex.
>>
>>
>> Signed-off-by: Gu Zheng <guz.fnst@cn.fujitsu.com>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/pci-sysfs.c |   13 ++++++++++---
>>  1 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
>> index 5b4a9d9..18590c1 100644
>> --- a/drivers/pci/pci-sysfs.c
>> +++ b/drivers/pci/pci-sysfs.c
>> @@ -328,10 +328,17 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
>>
>>  static void remove_callback(struct device *dev)
>>  {
>> -       struct pci_dev *pdev = to_pci_dev(dev);
>> -
>> +       struct pci_dev *pdev = to_pci_dev(dev), *tmp;
>> +       bool found = false;
>> +       struct pci_bus *bus = pdev->bus;
>>         mutex_lock(&pci_remove_rescan_mutex);
>> -       pci_stop_and_remove_bus_device(pdev);
>> +       list_for_each_entry(tmp, &bus->devices, bus_list)
>> +               if (tmp == pdev) {
>> +                       found = true;
>> +                       break;
>> +               }
>> +       if (found)
>> +               pci_stop_and_remove_bus_device(pdev);
> 
> I assume this series won't be needed when we eventually get the
> reference counting sorted out, because we shouldn't have to check
> whether the device still exists.  

Yes, your are right! Thanks for your correction.

>Yinghai posted a
> "fix_racing_removing_5.patch", which currently doesn't apply cleanly,
> but I assume he'll refresh it after v3.10-rc1 is out.

OK, let's wait and see! :)

Best regards,
Gu

> 
> Bjorn
> 
>>         mutex_unlock(&pci_remove_rescan_mutex);
>>  }
>>
>> --
>> 1.7.7
>>
>