* [PATCH v2 1/4] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev()
[not found] ` <CAErSpo65ARR_fZwsGCZDJqbPWktHYUkufgPt-2mWwur=nPR1Qg@mail.gmail.com>
@ 2013-04-30 11:31 ` Gu Zheng
2013-04-30 11:31 ` [PATCH v2 2/4] PCI: introduce pci_bus_get()/pci_bus_put() to hide pci_bus' reference management Gu Zheng
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Gu Zheng @ 2013-04-30 11:31 UTC (permalink / raw)
To: Bjorn Helgaas, Yinghai Lu
Cc: linux-pci@vger.kernel.org, Yasuaki Ishimatsu, Taku Izumi,
Jiang Liu, tangchen, Lin Feng, linux-kernel, guz.fnst
>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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/4] PCI: introduce pci_bus_get()/pci_bus_put() to hide pci_bus' reference management
[not found] ` <CAErSpo65ARR_fZwsGCZDJqbPWktHYUkufgPt-2mWwur=nPR1Qg@mail.gmail.com>
2013-04-30 11:31 ` [PATCH v2 1/4] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev() Gu Zheng
@ 2013-04-30 11:31 ` Gu Zheng
2013-04-30 11:31 ` [PATCH v2 3/4] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead Gu Zheng
2013-04-30 11:31 ` [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback() Gu Zheng
3 siblings, 0 replies; 6+ messages in thread
From: Gu Zheng @ 2013-04-30 11:31 UTC (permalink / raw)
To: Bjorn Helgaas, Yinghai Lu
Cc: linux-pci@vger.kernel.org, Yasuaki Ishimatsu, Taku Izumi,
Jiang Liu, tangchen, Lin Feng, linux-kernel, guz.fnst
>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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/4] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead
[not found] ` <CAErSpo65ARR_fZwsGCZDJqbPWktHYUkufgPt-2mWwur=nPR1Qg@mail.gmail.com>
2013-04-30 11:31 ` [PATCH v2 1/4] PCI: Introduce pci_alloc_dev(struct pci_bus*) to replace alloc_pci_dev() Gu Zheng
2013-04-30 11:31 ` [PATCH v2 2/4] PCI: introduce pci_bus_get()/pci_bus_put() to hide pci_bus' reference management Gu Zheng
@ 2013-04-30 11:31 ` Gu Zheng
2013-04-30 11:31 ` [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback() Gu Zheng
3 siblings, 0 replies; 6+ messages in thread
From: Gu Zheng @ 2013-04-30 11:31 UTC (permalink / raw)
To: Bjorn Helgaas, Yinghai Lu
Cc: linux-pci@vger.kernel.org, Yasuaki Ishimatsu, Taku Izumi,
Jiang Liu, tangchen, Lin Feng, linux-kernel, guz.fnst
>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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback()
[not found] ` <CAErSpo65ARR_fZwsGCZDJqbPWktHYUkufgPt-2mWwur=nPR1Qg@mail.gmail.com>
` (2 preceding siblings ...)
2013-04-30 11:31 ` [PATCH v2 3/4] PCI: Convert alloc_pci_dev(void) to pci_alloc_dev(bus) instead Gu Zheng
@ 2013-04-30 11:31 ` Gu Zheng
2013-05-08 22:32 ` Bjorn Helgaas
3 siblings, 1 reply; 6+ messages in thread
From: Gu Zheng @ 2013-04-30 11:31 UTC (permalink / raw)
To: Bjorn Helgaas, Yinghai Lu
Cc: linux-pci@vger.kernel.org, linux-kernel, Yasuaki Ishimatsu,
Taku Izumi, tangchen, 'Lin Feng', Jiang Liu, guz.fnst
>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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback()
2013-04-30 11:31 ` [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback() Gu Zheng
@ 2013-05-08 22:32 ` Bjorn Helgaas
2013-05-09 2:23 ` Gu Zheng
0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2013-05-08 22:32 UTC (permalink / raw)
To: Gu Zheng
Cc: Yinghai Lu, linux-pci@vger.kernel.org, linux-kernel,
Yasuaki Ishimatsu, Taku Izumi, tangchen, Lin Feng, Jiang Liu
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 4/4] PCI: Check if the pci device get removed from pci tree already in remove_callback()
2013-05-08 22:32 ` Bjorn Helgaas
@ 2013-05-09 2:23 ` Gu Zheng
0 siblings, 0 replies; 6+ messages in thread
From: Gu Zheng @ 2013-05-09 2:23 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Yinghai Lu, linux-pci@vger.kernel.org, linux-kernel,
Yasuaki Ishimatsu, Taku Izumi, tangchen, Lin Feng, Jiang Liu
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
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread