All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem
@ 2012-03-23 14:58 Jiang Liu
  2012-03-23 14:58 ` [PATCH 01/11] PCI: Fix device reference count leakage in pci_dev_present() Jiang Liu
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes, Len Brown
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

This is a series of RFC patches, to make sure we are doing the
right thing in the right way.  The patchset hasn't been tested yet.
There are several minor bugfixes and two proposals.

The first proposal is to add notification chain for PCI hotplug events,
so other components interested in PCI root/bus/device hotplug events
could subscribe to the chain.

The second prososal is to introduce a recursive mutex lock to
serialize all hotplug operations triggered by sysfs interfaces,
PCI HPC hardware events, ACPI hotplug events and other sources.

There are still works left in driver/pci/hotplug directory to apply
the above two proposals.

The patchset applies to Yinghai's work on PCI root bus hotplug at
git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-irq
And it depends on my previous patchset, please refer to
http://www.spinics.net/lists/linux-pci/msg14472.html

Thanks!

Jiang Liu (11):
  PCI: Fix device reference count leakage in pci_dev_present()
  PCI: introduce pci_bus_get()/pci_bus_put() to hide implementation
    details
  PCI: clean up root bridge related logic in acpiphp driver
  ACPI,PCI: fix race windows caused by alloc_acpi_hotplug_work()
  PCI: Add notification interfaces for PCI root/bus/device hotplug
    events
  ACPI,PCI: update ACPI<->PCI binding information when pci hotplug
    event happens
  ACPI,PCI: update ACPI slots when PCI hotplug event happens
  PCI: Introduce recursive mutex to serialize PCI hotplug operations
  PCI: serialize hotplug operations triggered by PCI hotplug sysfs
    interfaces
  PCI,ACPI: serialize hotplug operations triggered by ACPI subsystem
  PCI: Serialize hotplug operations triggered by acpiphp driver

 drivers/acpi/pci_bind.c                |  115 +++++++++++++++++++++++++
 drivers/acpi/pci_root.c                |    8 ++
 drivers/acpi/pci_slot.c                |   81 ++++++++++++++++++-
 drivers/pci/bus.c                      |   20 ++++-
 drivers/pci/hotplug.c                  |   47 +++++++++++
 drivers/pci/hotplug/acpiphp.h          |    7 ++-
 drivers/pci/hotplug/acpiphp_glue.c     |  143 ++++++++++++--------------------
 drivers/pci/hotplug/pci_hotplug_core.c |    2 +
 drivers/pci/pci-sysfs.c                |   29 +++----
 drivers/pci/pci.h                      |    8 ++
 drivers/pci/probe.c                    |   10 ++-
 drivers/pci/remove.c                   |   20 ++++-
 drivers/pci/search.c                   |   10 +-
 include/linux/pci.h                    |   57 +++++++++++++
 14 files changed, 437 insertions(+), 120 deletions(-)

-- 
1.7.5.4


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 01/11] PCI: Fix device reference count leakage in pci_dev_present()
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 02/11] PCI: introduce pci_bus_get()/pci_bus_put() to hide implementation details Jiang Liu
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

This patch fixes a pci device reference count leakage issue in function
pci_dev_present().

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/pci/search.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 9d75dc8..b572730 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -338,13 +338,13 @@ int pci_dev_present(const struct pci_device_id *ids)
 	WARN_ON(in_interrupt());
 	while (ids->vendor || ids->subvendor || ids->class_mask) {
 		found = pci_get_dev_by_id(ids, NULL);
-		if (found)
-			goto exit;
+		if (found) {
+			pci_dev_put(found);
+			return 1;
+		}
 		ids++;
 	}
-exit:
-	if (found)
-		return 1;
+
 	return 0;
 }
 EXPORT_SYMBOL(pci_dev_present);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 02/11] PCI: introduce pci_bus_get()/pci_bus_put() to hide implementation details
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
  2012-03-23 14:58 ` [PATCH 01/11] PCI: Fix device reference count leakage in pci_dev_present() Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 03/11] PCI: clean up root bridge related logic in acpiphp driver Jiang Liu
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

Introduce pci_bus_get()/pci_bus_put() to hide implementation details

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/acpi/pci_slot.c            |    4 ++--
 drivers/pci/bus.c                  |   15 +++++++++++++++
 drivers/pci/hotplug/acpiphp_glue.c |    4 ++--
 include/linux/pci.h                |    2 ++
 4 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index e50e31a..323797a 100644
--- a/drivers/acpi/pci_slot.c
+++ b/drivers/acpi/pci_slot.c
@@ -166,7 +166,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 	list_add(&slot->list, &slot_list);
 	mutex_unlock(&slot_list_lock);
 
-	get_device(&pci_bus->dev);
+	pci_bus_get(pci_bus);
 
 	dbg("pci_slot: %p, pci_bus: %x, device: %d, name: %s\n",
 		pci_slot, pci_bus->number, device, name);
@@ -322,7 +322,7 @@ acpi_pci_slot_remove(acpi_handle handle)
 			list_del(&slot->list);
 			pbus = slot->pci_slot->bus;
 			pci_destroy_slot(slot->pci_slot);
-			put_device(&pbus->dev);
+			pci_bus_put(pbus);
 			kfree(slot);
 		}
 	}
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 672ffc8..2803120 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -35,6 +35,21 @@ void pci_add_resource_offset(struct list_head *resources, struct resource *res,
 }
 EXPORT_SYMBOL(pci_add_resource_offset);
 
+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(struct list_head *resources, struct resource *res)
 {
 	pci_add_resource_offset(resources, res, 0);
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 9d781a7..11155ef 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -397,7 +397,7 @@ static void add_p2p_bridge(acpi_handle *handle)
 	 * removed via PCI core logical hotplug. The ref pins the bus
 	 * (which we access during module unload).
 	 */
-	get_device(&bridge->pci_bus->dev);
+	pci_bus_get(bridge->pci_bus);
 
 	init_bridge_misc(bridge);
 	return;
@@ -527,7 +527,7 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
 	 * Only P2P bridges have a pci_dev
 	 */
 	if (bridge->pci_dev)
-		put_device(&bridge->pci_bus->dev);
+		pci_bus_put(bridge->pci_bus);
 
 	pci_dev_put(bridge->pci_dev);
 	list_del(&bridge->list);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 161f6c0..427ea42 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -933,6 +933,8 @@ 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 */
+struct pci_bus *pci_bus_get(struct pci_bus *bus);
+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.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 03/11] PCI: clean up root bridge related logic in acpiphp driver
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
  2012-03-23 14:58 ` [PATCH 01/11] PCI: Fix device reference count leakage in pci_dev_present() Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 02/11] PCI: introduce pci_bus_get()/pci_bus_put() to hide implementation details Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 04/11] ACPI,PCI: fix race windows caused by alloc_acpi_hotplug_work() Jiang Liu
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

Changeset
4f30793 PCI, acpiphp: Separate out hot-add support of pci host bridge
tries to remove all PCI host bridge related code from acpiphp driver.
There's still some code for PCI host bridge hotplug left in
acpiphp_glue.c, clean it up.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/pci/hotplug/acpiphp_glue.c |   89 ++----------------------------------
 1 files changed, 4 insertions(+), 85 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 11155ef..003a6d5 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1040,67 +1040,9 @@ static void acpiphp_sanitize_bus(struct pci_bus *bus)
 	}
 }
 
-/* Program resources in newly inserted bridge */
-static int acpiphp_configure_bridge (acpi_handle handle)
-{
-	struct pci_bus *bus;
-	struct pci_dev *pdev = acpi_get_pci_dev(handle);
-
-	bus = pdev->subordinate;
-	pci_dev_put(pdev);
-
-	pci_bus_size_bridges(bus);
-	pci_bus_assign_resources(bus);
-	acpiphp_sanitize_bus(bus);
-	acpiphp_set_hpp_values(bus);
-	pci_enable_bridges(bus);
-	return 0;
-}
-
-static void handle_bridge_insertion(acpi_handle handle, u32 type)
-{
-	struct acpi_device *device, *pdevice;
-	acpi_handle phandle;
-
-	if ((type != ACPI_NOTIFY_BUS_CHECK) &&
-			(type != ACPI_NOTIFY_DEVICE_CHECK)) {
-		err("unexpected notification type %d\n", type);
-		return;
-	}
-
-	acpi_get_parent(handle, &phandle);
-	if (acpi_bus_get_device(phandle, &pdevice)) {
-		dbg("no parent device, assuming NULL\n");
-		pdevice = NULL;
-	}
-	if (acpi_bus_add(&device, pdevice, handle, ACPI_BUS_TYPE_DEVICE)) {
-		err("cannot add bridge to acpi list\n");
-		return;
-	}
-	if (!acpiphp_configure_bridge(handle) &&
-		!acpi_bus_start(device))
-		add_bridge(handle);
-	else
-		err("cannot configure and start bridge\n");
-
-}
-
 /*
  * ACPI event handlers
  */
-
-static acpi_status
-count_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
-{
-	int *count = (int *)context;
-	struct acpiphp_bridge *bridge;
-
-	bridge = acpiphp_handle_to_bridge(handle);
-	if (bridge)
-		(*count)++;
-	return AE_OK ;
-}
-
 static acpi_status
 check_sub_bridges(acpi_handle handle, u32 lvl, void *context, void **rv)
 {
@@ -1125,8 +1067,6 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 	char objname[64];
 	struct acpi_buffer buffer = { .length = sizeof(objname),
 				      .pointer = objname };
-	struct acpi_device *device;
-	int num_sub_bridges = 0;
 	struct acpi_hp_work *hp_work;
 	acpi_handle handle;
 	u32 type;
@@ -1134,23 +1074,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 	hp_work = container_of(work, struct acpi_hp_work, work);
 	handle = hp_work->handle;
 	type = hp_work->type;
-
-	if (acpi_bus_get_device(handle, &device)) {
-		/* This bridge must have just been physically inserted */
-		handle_bridge_insertion(handle, type);
-		goto out;
-	}
-
-	bridge = acpiphp_handle_to_bridge(handle);
-	if (type == ACPI_NOTIFY_BUS_CHECK) {
-		acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, ACPI_UINT32_MAX,
-			count_sub_bridges, NULL, &num_sub_bridges, NULL);
-	}
-
-	if (!bridge && !num_sub_bridges) {
-		err("cannot get bridge info\n");
-		goto out;
-	}
+	bridge = (struct acpiphp_bridge *)hp_work->context;
 
 	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 
@@ -1163,9 +1087,8 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 				__func__, objname);
 			acpiphp_check_bridge(bridge);
 		}
-		if (num_sub_bridges)
-			acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
-				ACPI_UINT32_MAX, check_sub_bridges, NULL, NULL, NULL);
+		acpi_walk_namespace(ACPI_TYPE_DEVICE, handle,
+			ACPI_UINT32_MAX, check_sub_bridges, NULL, NULL, NULL);
 		break;
 
 	case ACPI_NOTIFY_DEVICE_CHECK:
@@ -1210,7 +1133,6 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 		break;
 	}
 
-out:
 	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
 }
 
@@ -1245,17 +1167,14 @@ static void _handle_hotplug_event_func(struct work_struct *work)
 	struct acpi_hp_work *hp_work;
 	acpi_handle handle;
 	u32 type;
-	void *context;
 
 	hp_work = container_of(work, struct acpi_hp_work, work);
 	handle = hp_work->handle;
 	type = hp_work->type;
-	context = hp_work->context;
+	func = (struct acpiphp_func *)hp_work->context;
 
 	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 
-	func = (struct acpiphp_func *)context;
-
 	switch (type) {
 	case ACPI_NOTIFY_BUS_CHECK:
 		/* bus re-enumerate */
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 04/11] ACPI,PCI: fix race windows caused by alloc_acpi_hotplug_work()
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
                   ` (2 preceding siblings ...)
  2012-03-23 14:58 ` [RFC PATCH 03/11] PCI: clean up root bridge related logic in acpiphp driver Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 05/11] PCI: Add notification interfaces for PCI root/bus/device hotplug events Jiang Liu
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes, Len Brown
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

alloc_acpi_hotplug_work() is used to move event handler from
kacpi_notify_wq to kacpi_hotplug_wq, so the event handler could
call acpi_remove_notify_handler() to unreigster itself, otherwise
it will deaklock. But alloc_acpi_hotplug_work() mechanism introduces
a small race window as below:
1. kacpi_notify_wq invokes the wrapper event handler, which enqueues
   the real event handler onto kacpi_hotplug_wq by calling
   alloc_acpi_hotplug_work().
2. Another thread calls acpi_remove_notify_handler() and releases
   all resources used by the real event handler.
3. kacpi_hotplug_wq invokes the real event handler, which may access
   the already released resources now.

This patch tries to close the small race window.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/pci/hotplug/acpiphp.h      |    7 ++++-
 drivers/pci/hotplug/acpiphp_glue.c |   50 +++++++++++++++++++++++++++++++----
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp.h b/drivers/pci/hotplug/acpiphp.h
index 1a62e7b..ca2da17 100644
--- a/drivers/pci/hotplug/acpiphp.h
+++ b/drivers/pci/hotplug/acpiphp.h
@@ -37,6 +37,7 @@
 
 #include <linux/acpi.h>
 #include <linux/mutex.h>
+#include <linux/kref.h>
 #include <linux/pci_hotplug.h>
 
 #define dbg(format, arg...)					\
@@ -74,6 +75,7 @@ static inline const char *slot_name(struct slot *slot)
 struct acpiphp_bridge {
 	struct list_head list;
 	acpi_handle handle;
+	struct kref kref;
 	struct acpiphp_slot *slots;
 
 	/* Ejectable PCI-to-PCI bridge (PCI bridge and PCI function) */
@@ -124,6 +126,7 @@ struct acpiphp_func {
 	struct list_head sibling;
 	struct notifier_block nb;
 	acpi_handle	handle;
+	struct kref	kref;
 
 	u8		function;	/* pci function# */
 	u32		flags;		/* see below */
@@ -160,6 +163,7 @@ struct acpiphp_attention_info
 #define BRIDGE_HAS_PS1		(0x00000020)
 #define BRIDGE_HAS_PS2		(0x00000040)
 #define BRIDGE_HAS_PS3		(0x00000080)
+#define BRIDGE_IS_DEAD		(0x80000000)
 
 /* slot flags */
 
@@ -175,7 +179,8 @@ struct acpiphp_attention_info
 #define FUNC_HAS_PS1		(0x00000020)
 #define FUNC_HAS_PS2		(0x00000040)
 #define FUNC_HAS_PS3		(0x00000080)
-#define FUNC_HAS_DCK            (0x00000100)
+#define FUNC_HAS_DCK		(0x00000100)
+#define FUNC_IS_DEAD		(0x80000000)
 
 /* function prototypes */
 
diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 003a6d5..764891e 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -115,6 +115,14 @@ static const struct acpi_dock_ops acpiphp_dock_ops = {
 	.handler = handle_hotplug_event_func,
 };
 
+static void acpiphp_release_func(struct kref *kref)
+{
+	struct acpiphp_func *func;
+
+	func = container_of(kref, struct acpiphp_func, kref);
+	kfree(func);
+}
+
 /* callback routine to register each ACPI PCI slot object */
 static acpi_status
 register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
@@ -153,6 +161,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 		return AE_NO_MEMORY;
 
 	INIT_LIST_HEAD(&newfunc->sibling);
+	kref_init(&newfunc->kref);
 	newfunc->handle = handle;
 	newfunc->function = function;
 
@@ -191,7 +200,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 	if (!slot) {
 		slot = kzalloc(sizeof(struct acpiphp_slot), GFP_KERNEL);
 		if (!slot) {
-			kfree(newfunc);
+			kref_put(&newfunc->kref, acpiphp_release_func);
 			return AE_NO_MEMORY;
 		}
 
@@ -266,7 +275,7 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 	bridge->nr_slots--;
 	bridge->slots = slot->next;
 	kfree(slot);
-	kfree(newfunc);
+	kref_put(&newfunc->kref, acpiphp_release_func);
 
 	return AE_OK;
 }
@@ -371,8 +380,16 @@ static inline void config_p2p_bridge_flags(struct acpiphp_bridge *bridge)
 	}
 }
 
+static void acpiphp_release_bridge(struct kref *kref)
+{
+	struct acpiphp_bridge *bridge;
+
+	bridge = container_of(kref, struct acpiphp_bridge, kref);
+	kfree(bridge);
+}
+
 /* allocate and initialize PCI-to-PCI bridge data structure */
-static void add_p2p_bridge(acpi_handle *handle)
+static void add_p2p_bridge(acpi_handle handle)
 {
 	struct acpiphp_bridge *bridge;
 
@@ -382,6 +399,7 @@ static void add_p2p_bridge(acpi_handle *handle)
 		return;
 	}
 
+	kref_init(&bridge->kref);
 	bridge->handle = handle;
 	config_p2p_bridge_flags(bridge);
 
@@ -403,7 +421,7 @@ static void add_p2p_bridge(acpi_handle *handle)
 	return;
  err:
 	pci_dev_put(bridge->pci_dev);
-	kfree(bridge);
+	kref_put(&bridge->kref, acpiphp_release_bridge);
 	return;
 }
 
@@ -515,7 +533,8 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
 					err("failed to remove notify handler\n");
 			}
 			list_del(&func->sibling);
-			kfree(func);
+			func->flags |= FUNC_IS_DEAD;
+			kref_put(&func->kref, acpiphp_release_func);
 		}
 		acpiphp_unregister_hotplug_slot(slot);
 		list_del(&slot->funcs);
@@ -531,7 +550,8 @@ static void cleanup_bridge(struct acpiphp_bridge *bridge)
 
 	pci_dev_put(bridge->pci_dev);
 	list_del(&bridge->list);
-	kfree(bridge);
+	bridge->flags |= BRIDGE_IS_DEAD;
+	kref_put(&bridge->kref, acpiphp_release_bridge);
 }
 
 static acpi_status
@@ -1075,6 +1095,8 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 	handle = hp_work->handle;
 	type = hp_work->type;
 	bridge = (struct acpiphp_bridge *)hp_work->context;
+	if (bridge->flags & BRIDGE_IS_DEAD)
+		goto out;
 
 	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 
@@ -1133,6 +1155,8 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 		break;
 	}
 
+out:
+	kref_put(&bridge->kref, acpiphp_release_bridge);
 	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
 }
 
@@ -1147,6 +1171,11 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 static void handle_hotplug_event_bridge(acpi_handle handle, u32 type,
 					void *context)
 {
+	struct acpiphp_bridge *bridge = (struct acpiphp_bridge *)context;
+
+	/* Will be released by _handle_hotplug_event_bridge() */
+	kref_get(&bridge->kref);
+
 	/*
 	 * Currently the code adds all hotplug events to the kacpid_wq
 	 * queue when it should add hotplug events to the kacpi_hotplug_wq.
@@ -1172,6 +1201,8 @@ static void _handle_hotplug_event_func(struct work_struct *work)
 	handle = hp_work->handle;
 	type = hp_work->type;
 	func = (struct acpiphp_func *)hp_work->context;
+	if (func->flags & FUNC_IS_DEAD)
+		goto out;
 
 	acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 
@@ -1205,6 +1236,8 @@ static void _handle_hotplug_event_func(struct work_struct *work)
 		break;
 	}
 
+out:
+	kref_put(&func->kref, acpiphp_release_func);
 	kfree(hp_work); /* allocated in handle_hotplug_event_func */
 }
 
@@ -1219,6 +1252,11 @@ static void _handle_hotplug_event_func(struct work_struct *work)
 static void handle_hotplug_event_func(acpi_handle handle, u32 type,
 				      void *context)
 {
+	struct acpiphp_func *func = (struct acpiphp_func *)context;
+
+	/* Will be released by _handle_hotplug_event_func() */
+	kref_get(&func->kref);
+
 	/*
 	 * Currently the code adds all hotplug events to the kacpid_wq
 	 * queue when it should add hotplug events to the kacpi_hotplug_wq.
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 05/11] PCI: Add notification interfaces for PCI root/bus/device hotplug events
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
                   ` (3 preceding siblings ...)
  2012-03-23 14:58 ` [RFC PATCH 04/11] ACPI,PCI: fix race windows caused by alloc_acpi_hotplug_work() Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 06/11] ACPI,PCI: update ACPI<->PCI binding information when pci hotplug event happens Jiang Liu
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

When PCI root/bus/device hotplug event happens, some other components
may need to get notified in order to update their states.  For example,
the ACPI<->PCI bind logic in drivers/acpi/pci_bind.c needs to update the
binding relationship when PCI hotplug event happens. Another example is,
the PCIe AER driver needs to setup the AER configuration for newly added
PCI devices. So introduce a blocking notify chain for PCI hotplug events.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/pci/bus.c     |    5 +++-
 drivers/pci/hotplug.c |   21 ++++++++++++++++++++
 drivers/pci/pci.h     |    8 +++++++
 drivers/pci/probe.c   |   10 ++++++++-
 drivers/pci/remove.c  |   20 ++++++++++++++----
 include/linux/pci.h   |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 108 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 2803120..61b8603 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -188,6 +188,8 @@ int pci_bus_add_device(struct pci_dev *dev)
 	dev->is_added = 1;
 	pci_proc_attach_device(dev);
 	pci_create_sysfs_dev_files(dev);
+	pci_call_hotplug_notifier(PCI_HP_EVENT_DEV_START, dev);
+
 	return 0;
 }
 
@@ -212,8 +214,9 @@ int pci_bus_add_child(struct pci_bus *bus)
 
 	/* Create legacy_io and legacy_mem files for this bus */
 	pci_create_legacy_files(bus);
+	pci_call_hotplug_notifier(PCI_HP_EVENT_BUS_START, bus);
 
-	return retval;
+	return 0;
 }
 
 /**
diff --git a/drivers/pci/hotplug.c b/drivers/pci/hotplug.c
index 2b5352a..df98119 100644
--- a/drivers/pci/hotplug.c
+++ b/drivers/pci/hotplug.c
@@ -1,8 +1,29 @@
 #include <linux/kernel.h>
 #include <linux/pci.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include "pci.h"
 
+static BLOCKING_NOTIFIER_HEAD(pci_hotplug_notify_chain);
+
+int pci_register_hotplug_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&pci_hotplug_notify_chain, nb);
+}
+EXPORT_SYMBOL(pci_register_hotplug_notifier);
+
+void pci_unregister_hotplug_notifier(struct notifier_block *nb)
+{
+	blocking_notifier_chain_unregister(&pci_hotplug_notify_chain, nb);
+}
+EXPORT_SYMBOL(pci_unregister_hotplug_notifier);
+
+int pci_call_hotplug_notifier(int event, void *data)
+{
+	return blocking_notifier_call_chain(&pci_hotplug_notify_chain,
+		event, data);
+}
+
 int pci_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct pci_dev *pdev;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9378d81..f0f9252 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -126,6 +126,14 @@ static inline int pci_proc_detach_bus(struct pci_bus *bus) { return 0; }
 /* Functions for PCI Hotplug drivers to use */
 int pci_hp_add_bridge(struct pci_dev *dev);
 extern unsigned int pci_do_scan_bus(struct pci_bus *bus);
+#ifdef	CONFIG_HOTPLUG
+extern int pci_call_hotplug_notifier(int event, void *data);
+#else
+static inline int pci_call_hotplug_notifier(int event, void *data)
+{
+	return NOTIFY_DONE;
+}
+#endif
 
 #ifdef HAVE_PCI_LEGACY
 extern void pci_create_legacy_files(struct pci_bus *bus);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index d626c4e..edbc013 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -651,6 +651,7 @@ struct pci_bus *__ref pci_add_new_bus(struct pci_bus *parent, struct pci_dev *de
 		down_write(&pci_bus_sem);
 		list_add_tail(&child->node, &parent->children);
 		up_write(&pci_bus_sem);
+		pci_call_hotplug_notifier(PCI_HP_EVENT_BUS_ADD, child);
 	}
 	return child;
 }
@@ -1472,6 +1473,8 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
 	down_write(&pci_bus_sem);
 	list_add_tail(&dev->bus_list, &bus->devices);
 	up_write(&pci_bus_sem);
+
+	pci_call_hotplug_notifier(PCI_HP_EVENT_DEV_ADD, dev);
 }
 
 struct pci_dev *__ref pci_scan_single_device(struct pci_bus *bus, int devfn)
@@ -1753,8 +1756,10 @@ static unsigned int __devinit __pci_scan_child_bus(struct pci_bus *bus,
 	if (!bus->is_added) {
 		dev_dbg(&bus->dev, "fixups for bus pass %d\n", pass);
 		pcibios_fixup_bus(bus);
-		if (pci_is_root_bus(bus))
+		if (pci_is_root_bus(bus)) {
 			bus->is_added = 1;
+			pci_call_hotplug_notifier(PCI_HP_EVENT_BUS_START, bus);
+		}
 	}
 
 	list_for_each_entry(dev, &bus->devices, bus_list)
@@ -1876,6 +1881,9 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 	list_add_tail(&b->node, &pci_root_buses);
 	up_write(&pci_bus_sem);
 
+	pci_call_hotplug_notifier(PCI_HP_EVENT_ROOT_ADD, bridge);
+	pci_call_hotplug_notifier(PCI_HP_EVENT_BUS_ADD, b);
+
 	return b;
 
 class_dev_reg_err:
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 18efb31..2b6dd97 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -20,6 +20,7 @@ static void pci_free_resources(struct pci_dev *dev)
 static void pci_stop_dev(struct pci_dev *dev)
 {
 	if (dev->is_added) {
+		pci_call_hotplug_notifier(PCI_HP_EVENT_DEV_STOP, dev);
 		pci_proc_detach_device(dev);
 		pci_remove_sysfs_dev_files(dev);
 		device_unregister(&dev->dev);
@@ -32,6 +33,8 @@ static void pci_stop_dev(struct pci_dev *dev)
 
 static void pci_destroy_dev(struct pci_dev *dev)
 {
+	pci_call_hotplug_notifier(PCI_HP_EVENT_DEV_REMOVE, dev);
+
 	/* Remove the device from the device lists, and prevent any further
 	 * list accesses from this device */
 	down_write(&pci_bus_sem);
@@ -64,16 +67,21 @@ int pci_remove_device_safe(struct pci_dev *dev)
 
 void pci_remove_bus(struct pci_bus *pci_bus)
 {
-	pci_proc_detach_bus(pci_bus);
+	if (pci_bus->is_added)
+		pci_call_hotplug_notifier(PCI_HP_EVENT_BUS_STOP, pci_bus);
 
-	down_write(&pci_bus_sem);
-	list_del(&pci_bus->node);
-	pci_bus_release_busn_res(pci_bus);
-	up_write(&pci_bus_sem);
 	if (pci_bus->is_added || pci_is_root_bus(pci_bus)) {
 		pci_remove_legacy_files(pci_bus);
 		device_unregister(&pci_bus->dev);
+		pci_bus->is_added = 0;
 	}
+
+	pci_call_hotplug_notifier(PCI_HP_EVENT_BUS_REMOVE, pci_bus);
+	pci_proc_detach_bus(pci_bus);
+	down_write(&pci_bus_sem);
+	list_del(&pci_bus->node);
+	pci_bus_release_busn_res(pci_bus);
+	up_write(&pci_bus_sem);
 }
 EXPORT_SYMBOL(pci_remove_bus);
 
@@ -171,8 +179,10 @@ void pci_stop_bus_device(struct pci_dev *dev)
 
 static void pci_stop_host_bridge(struct pci_host_bridge *bridge)
 {
+	pci_call_hotplug_notifier(PCI_HP_EVENT_ROOT_REMOVE, bridge);
 	device_unregister(&bridge->dev);
 }
+
 /*
  * it will support pci root bus too, in that case we need
  *  stop and remove host bridge
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 427ea42..953e0ef 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -53,6 +53,7 @@
 #include <linux/device.h>
 #include <linux/io.h>
 #include <linux/irqreturn.h>
+#include <linux/notifier.h>
 
 /* Include the ID list */
 #include <linux/pci_ids.h>
@@ -903,6 +904,56 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge);
 unsigned int pci_rescan_bus(struct pci_bus *bus);
 #endif
 
+/*
+ * Notification event types for PCI root, bus and device hotplug.
+ */
+enum {
+	PCI_HP_EVENT_BUS_ADD = 0x1,	/* New bus (struct pci_bus *)v has
+					 * been created.
+					 */
+	PCI_HP_EVENT_BUS_REMOVE = 0x2,	/* Bus (struct pci_bus *)v will be
+					 * destroyed.
+					 */
+	PCI_HP_EVENT_BUS_START = 0x3,	/* Bus (struct pci_bus *)v has been
+					 * started.
+					 */
+	PCI_HP_EVENT_BUS_STOP = 0x4,	/* Bus (struct pci_bus *)v will be
+					 * stopped.
+					 */
+	PCI_HP_EVENT_DEV_ADD = 0x11,	/* New device (struct pci_dev *)v has
+					 * been created.
+					 */
+	PCI_HP_EVENT_DEV_REMOVE = 0x12,	/* Device (struct pci_dev *)v will be
+					 * destroyed.
+					 */
+	PCI_HP_EVENT_DEV_START = 0x13,	/* Device (struct pci_dev *)v has been
+					 * started.
+					 */
+	PCI_HP_EVENT_DEV_STOP = 0x14,	/* Device (struct pci_dev *)v will be
+					 * stopped.
+					 */
+	PCI_HP_EVENT_ROOT_ADD = 0x21,	/* Root (struct pci_host_bridge *)v
+					 * has been created.
+					 */
+	PCI_HP_EVENT_ROOT_REMOVE = 0x22,/* Root (struct pci_host_bridge *)v
+					 * will be destroyed.
+					 */
+};
+
+#ifdef CONFIG_HOTPLUG
+extern int pci_register_hotplug_notifier(struct notifier_block *nb);
+extern void pci_unregister_hotplug_notifier(struct notifier_block *nb);
+#else
+static inline int pci_register_hotplug_notifier(struct notifier_block *nb)
+{
+	return 0;
+}
+
+static inline void pci_unregister_hotplug_notifier(struct notifier_block *nb)
+{
+}
+#endif
+
 /* Vital product data routines */
 ssize_t pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
 ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 06/11] ACPI,PCI: update ACPI<->PCI binding information when pci hotplug event happens
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
                   ` (4 preceding siblings ...)
  2012-03-23 14:58 ` [RFC PATCH 05/11] PCI: Add notification interfaces for PCI root/bus/device hotplug events Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 07/11] ACPI,PCI: update ACPI slots when PCI " Jiang Liu
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes, Len Brown
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

When PCI hotplug event happens, the ACPI<->PCI binding relationship should be
updated. Otherwise the binding relationship may become stale and cause invalid
data access or wrong searching results. For example, thinking about following
sequences:
1. Function acpi_pci_bind() installs an ACPI PME event hanlder for the original
   PCI device (orig_device).
2. The orignal PCI device is replaced by a new PCI device, so orig_device
   destroyed and a new_device created.
3. The new PCI device triggers an ACPI PME event.
4. The registered PME event handle pci_acpi_wake_dev() still tries to access
   the orig_device, which may have already been released now.
5. Invalid memory access or event memory corruption.

The solution here is to update the binding relationship when PCI hotplug
event happens.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/acpi/pci_bind.c |  115 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 115 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/pci_bind.c b/drivers/acpi/pci_bind.c
index 2ef0409..1dd6379 100644
--- a/drivers/acpi/pci_bind.c
+++ b/drivers/acpi/pci_bind.c
@@ -118,3 +118,118 @@ int acpi_pci_bind_root(struct acpi_device *device)
 
 	return 0;
 }
+
+static struct acpi_device *
+acpi_pci_bind_notify_check(struct pci_dev *pci_dev)
+{
+	acpi_handle handle;
+	struct acpi_device *device;
+
+	handle = DEVICE_ACPI_HANDLE(&pci_dev->dev);
+	if (handle == NULL || acpi_bus_get_device(handle, &device))
+		return NULL;
+
+	if (device && device->parent && device->parent->ops.bind) 
+		return device;
+
+	return NULL;
+}
+
+static void acpi_pci_bind_notify_dev_add(struct pci_dev *pci_dev)
+{
+	struct acpi_device *device;
+
+	device = acpi_pci_bind_notify_check(pci_dev);
+	if (device) {
+		pci_acpi_add_pm_notifier(device, pci_dev);
+		if (device->wakeup.flags.run_wake)
+			device_set_run_wake(&pci_dev->dev, true);
+	}
+}
+
+static void acpi_pci_bind_notify_dev_remove(struct pci_dev *pci_dev)
+{
+	struct acpi_device *device;
+
+	device = acpi_pci_bind_notify_check(pci_dev);
+	if (device) {
+		device_set_run_wake(&pci_dev->dev, false);
+		pci_acpi_remove_pm_notifier(device);
+	}
+}
+
+static void acpi_pci_bind_notify_bus_add(struct pci_bus *bus)
+{
+	acpi_handle handle;
+	struct acpi_device *device;
+
+	/* Skip root buses */
+	if (!bus->self)
+		return;
+
+	device = acpi_pci_bind_notify_check(bus->self);
+	if (device) {
+		/*
+		 * Install the 'bind' function to facilitate callbacks for
+		 * children of the P2P bridge.
+		 */
+		device->ops.bind = acpi_pci_bind;
+		device->ops.unbind = acpi_pci_unbind;
+
+		/* Evaluate and parse _PRT, if exists. */
+		if (ACPI_SUCCESS(acpi_get_handle(device->handle,
+						 METHOD_NAME__PRT, &handle)))
+			acpi_pci_irq_add_prt(device->handle, bus);
+	}
+}
+
+static void acpi_pci_bind_notify_bus_remove(struct pci_bus *bus)
+{
+	struct acpi_device *device;
+
+	/* Skip root buses */
+	if (!bus->self)
+		return;
+
+	device = acpi_pci_bind_notify_check(bus->self);
+	if (device) {
+		acpi_pci_irq_del_prt(bus);
+
+		device->ops.bind = NULL;
+		device->ops.unbind = NULL;
+	}
+}
+
+static int acpi_pci_bind_notify_fn(struct notifier_block *nb,
+				   unsigned long event, void *data)
+{
+	switch (event) {
+	case PCI_HP_EVENT_DEV_ADD:
+		acpi_pci_bind_notify_dev_add(data);
+		break;
+	case PCI_HP_EVENT_DEV_REMOVE:
+		acpi_pci_bind_notify_dev_remove(data);
+		break;
+	case PCI_HP_EVENT_BUS_ADD:
+		acpi_pci_bind_notify_bus_add(data);
+		break;
+	case PCI_HP_EVENT_BUS_REMOVE:
+		acpi_pci_bind_notify_bus_remove(data);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_pci_bind_notifier = {
+	.notifier_call = &acpi_pci_bind_notify_fn,
+};
+
+static int __init acpi_pci_bind_init(void)
+{
+	return pci_register_hotplug_notifier(&acpi_pci_bind_notifier);
+}
+
+subsys_initcall(acpi_pci_bind_init);
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 07/11] ACPI,PCI: update ACPI slots when PCI hotplug event happens
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
                   ` (5 preceding siblings ...)
  2012-03-23 14:58 ` [RFC PATCH 06/11] ACPI,PCI: update ACPI<->PCI binding information when pci hotplug event happens Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 08/11] PCI: Introduce recursive mutex to serialize PCI hotplug operations Jiang Liu
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes, Len Brown
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

When a PCI bus is removed, all ACPI slots associated with that
bus should be destroyed. Otherwise the ACPI slot device may
cause invalide memory access to the destroyed PCI bus.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/acpi/pci_slot.c |   77 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 77 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/pci_slot.c b/drivers/acpi/pci_slot.c
index 323797a..0b67c13 100644
--- a/drivers/acpi/pci_slot.c
+++ b/drivers/acpi/pci_slot.c
@@ -32,6 +32,7 @@
 #include <linux/acpi.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
+#include <linux/pci-acpi.h>
 #include <linux/dmi.h>
 
 static bool debug;
@@ -145,6 +146,21 @@ register_slot(acpi_handle handle, u32 lvl, void *context, void **rv)
 	if (device < 0)
 		return AE_OK;
 
+	/*
+	 * Slots may have already been created for PCI slots under
+	 * hot-added PCI host bridges.
+	 */
+	mutex_lock(&slot_list_lock);
+	list_for_each_entry(slot, &slot_list, list) {
+		pci_slot = slot->pci_slot;
+		if (pci_slot && pci_slot->bus == pci_bus &&
+		    pci_slot->number == device) {
+			mutex_unlock(&slot_list_lock);
+			return AE_OK;
+		}
+	}
+	mutex_unlock(&slot_list_lock);
+
 	slot = kmalloc(sizeof(*slot), GFP_KERNEL);
 	if (!slot) {
 		err("%s: cannot allocate memory\n", __func__);
@@ -354,17 +370,78 @@ static struct dmi_system_id acpi_pci_slot_dmi_table[] __initdata = {
 	{}
 };
 
+static void acpi_pci_slot_notify_bus_add(struct pci_bus *bus)
+{
+	acpi_handle handle;
+	struct callback_args context;
+
+	/* Skip root buses */
+	if (!bus->self)
+		return;
+
+	context.pci_bus = bus;
+	context.user_function = register_slot;
+	context.root_handle = acpi_find_root_bridge_handle(bus->self);
+	handle = acpi_pci_get_bridge_handle(bus);
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, handle, (u32)1,
+			    register_slot, NULL, &context, NULL);
+}
+
+static void acpi_pci_slot_notify_bus_remove(struct pci_bus *bus)
+{
+	struct acpi_pci_slot *slot, *tmp;
+
+	/* Skip root buses */
+	if (!bus->self)
+		return;
+
+	mutex_lock(&slot_list_lock);
+	list_for_each_entry_safe(slot, tmp, &slot_list, list) {
+		if (slot->pci_slot && slot->pci_slot->bus == bus) {
+			list_del(&slot->list);
+			pci_destroy_slot(slot->pci_slot);
+			put_device(&bus->dev);
+			kfree(slot);
+		}
+	}
+	mutex_unlock(&slot_list_lock);
+}
+
+static int acpi_pci_slot_notify_fn(struct notifier_block *nb,
+				   unsigned long event, void *data)
+{
+	switch (event) {
+	case PCI_HP_EVENT_BUS_ADD:
+		acpi_pci_slot_notify_bus_add(data);
+		break;
+	case PCI_HP_EVENT_BUS_REMOVE:
+		acpi_pci_slot_notify_bus_remove(data);
+		break;
+	default:
+		return NOTIFY_DONE;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block acpi_pci_slot_notifier = {
+	.notifier_call = &acpi_pci_slot_notify_fn,
+};
+
 static int __init
 acpi_pci_slot_init(void)
 {
 	dmi_check_system(acpi_pci_slot_dmi_table);
 	acpi_pci_register_driver(&acpi_pci_slot_driver);
+	pci_register_hotplug_notifier(&acpi_pci_slot_notifier);
+
 	return 0;
 }
 
 static void __exit
 acpi_pci_slot_exit(void)
 {
+	pci_unregister_hotplug_notifier(&acpi_pci_slot_notifier);
 	acpi_pci_unregister_driver(&acpi_pci_slot_driver);
 }
 
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 08/11] PCI: Introduce recursive mutex to serialize PCI hotplug operations
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
                   ` (6 preceding siblings ...)
  2012-03-23 14:58 ` [RFC PATCH 07/11] ACPI,PCI: update ACPI slots when PCI " Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 09/11] PCI: serialize hotplug operations triggered by PCI hotplug sysfs interfaces Jiang Liu
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

There are multiple entrances to trigger PCI hotplug operations:
1. Sysfs interface exported by PCI subsystem
2. PCI hotplug event triggered by PCI Hotplug Controller
3. ACPI hotplug event for PCI host bridge
4. Driver attach/detach event, such as pci_root driver

Currently pci_remove_rescan_mutex only serializes hotplug operations
triggered by method 1. This patch abstracts pci_remove_rescan_mutex
as pci_lock_hotplug()/pci_unlock_hotplug(), so it could be used to
serialize all PCI hotplug operations triggered by all methods above.
pci_lock_hotplug()/pci_unlock_hotplug() supports recursive lock.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/pci/hotplug.c   |   26 ++++++++++++++++++++++++++
 drivers/pci/pci-sysfs.c |   29 ++++++++++++++---------------
 include/linux/pci.h     |    4 ++++
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/hotplug.c b/drivers/pci/hotplug.c
index df98119..dc4d561 100644
--- a/drivers/pci/hotplug.c
+++ b/drivers/pci/hotplug.c
@@ -4,8 +4,34 @@
 #include <linux/notifier.h>
 #include "pci.h"
 
+/* Recursive mutex for PCI hotplug operations. */
+static DEFINE_MUTEX(pci_hotplug_mutex);
+static struct task_struct *pci_hotplug_mutex_owner;
+static int pci_hotplug_mutex_recursive;
+
 static BLOCKING_NOTIFIER_HEAD(pci_hotplug_notify_chain);
 
+void pci_lock_hotplug(void)
+{
+	if (current != pci_hotplug_mutex_owner) {
+		mutex_lock(&pci_hotplug_mutex);
+		pci_hotplug_mutex_owner = current;
+	}
+	pci_hotplug_mutex_recursive++;
+
+}
+EXPORT_SYMBOL(pci_lock_hotplug);
+
+void pci_unlock_hotplug(void)
+{
+	BUG_ON(pci_hotplug_mutex_owner != current);
+	if (--pci_hotplug_mutex_recursive == 0) {
+		pci_hotplug_mutex_owner = NULL;
+		mutex_unlock(&pci_hotplug_mutex);
+	}
+}
+EXPORT_SYMBOL(pci_unlock_hotplug);
+
 int pci_register_hotplug_notifier(struct notifier_block *nb)
 {
 	return blocking_notifier_chain_register(&pci_hotplug_notify_chain, nb);
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index c348048..bb97e8b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -283,7 +283,6 @@ msi_bus_store(struct device *dev, struct device_attribute *attr,
 }
 
 #ifdef CONFIG_HOTPLUG
-static DEFINE_MUTEX(pci_remove_rescan_mutex);
 static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
 				size_t count)
 {
@@ -294,10 +293,10 @@ static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
 		return -EINVAL;
 
 	if (val) {
-		mutex_lock(&pci_remove_rescan_mutex);
+		pci_lock_hotplug();
 		while ((b = pci_find_next_bus(b)) != NULL)
 			pci_rescan_bus(b);
-		mutex_unlock(&pci_remove_rescan_mutex);
+		pci_unlock_hotplug();
 	}
 	return count;
 }
@@ -313,9 +312,9 @@ static ssize_t bus_rescan_root_store(struct bus_type *bus, const char *buf,
 		return -EINVAL;
 
 	if (val) {
-		mutex_lock(&pci_remove_rescan_mutex);
+		pci_lock_hotplug();
 		arch_pci_root_rescan();
-		mutex_unlock(&pci_remove_rescan_mutex);
+		pci_unlock_hotplug();
 	}
 	return count;
 }
@@ -340,9 +339,9 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
 		printk(KERN_WARNING "rescan with pci device will be removed "
 			 "shortly, please use bridge rescan_bridge\n"
 			 "or bus/rescan instead\n");
-		mutex_lock(&pci_remove_rescan_mutex);
+		pci_lock_hotplug();
 		pci_rescan_bus(pdev->bus);
-		mutex_unlock(&pci_remove_rescan_mutex);
+		pci_unlock_hotplug();
 	}
 	return count;
 }
@@ -351,9 +350,9 @@ static void bridge_rescan_callback(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	mutex_lock(&pci_remove_rescan_mutex);
+	pci_lock_hotplug();
 	pci_rescan_bus_bridge_resize(pdev);
-	mutex_unlock(&pci_remove_rescan_mutex);
+	pci_unlock_hotplug();
 }
 
 static ssize_t
@@ -382,9 +381,9 @@ static void remove_callback(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 
-	mutex_lock(&pci_remove_rescan_mutex);
+	pci_lock_hotplug();
 	pci_stop_and_remove_bus_device(pdev);
-	mutex_unlock(&pci_remove_rescan_mutex);
+	pci_unlock_hotplug();
 }
 
 static ssize_t
@@ -414,9 +413,9 @@ static void bus_remove_callback(struct device *dev)
 {
 	struct pci_bus *bus = to_pci_bus(dev);
 
-	mutex_lock(&pci_remove_rescan_mutex);
+	pci_lock_hotplug();
 	pci_stop_and_remove_bus(bus);
-	mutex_unlock(&pci_remove_rescan_mutex);
+	pci_unlock_hotplug();
 }
 static ssize_t
 dev_bus_remove_store(struct device *dev, struct device_attribute *attr,
@@ -443,9 +442,9 @@ static void bus_rescan_callback(struct device *dev)
 {
 	struct pci_bus *bus = to_pci_bus(dev);
 
-	mutex_lock(&pci_remove_rescan_mutex);
+	pci_lock_hotplug();
 	pci_rescan_bus(bus);
-	mutex_unlock(&pci_remove_rescan_mutex);
+	pci_unlock_hotplug();
 }
 static ssize_t
 dev_bus_rescan_store(struct device *dev, struct device_attribute *attr,
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 953e0ef..8fa3ed5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -941,9 +941,13 @@ enum {
 };
 
 #ifdef CONFIG_HOTPLUG
+extern void pci_lock_hotplug(void);
+extern void pci_unlock_hotplug(void);
 extern int pci_register_hotplug_notifier(struct notifier_block *nb);
 extern void pci_unregister_hotplug_notifier(struct notifier_block *nb);
 #else
+static inline void pci_lock_hotplug(void) {}
+static inline  void pci_unlock_hotplug(void) {}
 static inline int pci_register_hotplug_notifier(struct notifier_block *nb)
 {
 	return 0;
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 09/11] PCI: serialize hotplug operations triggered by PCI hotplug sysfs interfaces
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
                   ` (7 preceding siblings ...)
  2012-03-23 14:58 ` [RFC PATCH 08/11] PCI: Introduce recursive mutex to serialize PCI hotplug operations Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 10/11] PCI,ACPI: serialize hotplug operations triggered by ACPI subsystem Jiang Liu
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes, Len Brown
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

Use pci_lock_hotplug()/pci_unlock_hotplug() to serialize hotplug operations
triggered by PCI hotplug sysfs interfaces.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/pci/hotplug/pci_hotplug_core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c
index 202f4a9..633fe3a 100644
--- a/drivers/pci/hotplug/pci_hotplug_core.c
+++ b/drivers/pci/hotplug/pci_hotplug_core.c
@@ -121,6 +121,7 @@ static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
 		retval = -ENODEV;
 		goto exit;
 	}
+	pci_lock_hotplug();
 	switch (power) {
 		case 0:
 			if (slot->ops->disable_slot)
@@ -136,6 +137,7 @@ static ssize_t power_write_file(struct pci_slot *pci_slot, const char *buf,
 			err ("Illegal value specified for power\n");
 			retval = -EINVAL;
 	}
+	pci_unlock_hotplug();
 	module_put(slot->ops->owner);
 
 exit:	
-- 
1.7.5.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 10/11] PCI,ACPI: serialize hotplug operations triggered by ACPI subsystem
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
                   ` (8 preceding siblings ...)
  2012-03-23 14:58 ` [RFC PATCH 09/11] PCI: serialize hotplug operations triggered by PCI hotplug sysfs interfaces Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-23 14:58 ` [RFC PATCH 11/11] PCI: Serialize hotplug operations triggered by acpiphp driver Jiang Liu
  2012-03-27  3:33 ` [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Kenji Kaneshige
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes, Len Brown
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

Use pci_lock_hotplug()/pci_unlock_hotplug() to serialize PCI
hotplug operations related to ACPI root bridges.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/acpi/pci_root.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 24afd66..a8a3867 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -463,6 +463,8 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 	if (!root)
 		return -ENOMEM;
 
+	pci_lock_hotplug();
+
 	segment = 0;
 	status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL,
 				       &segment);
@@ -627,12 +629,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
 			driver->add(device->handle);
 
 	mutex_unlock(&acpi_pci_root_lock);
+	pci_unlock_hotplug();
 
 	return 0;
 
 out_del_root:
 	list_del_rcu(&root->node);
 	mutex_unlock(&acpi_pci_root_lock);
+	pci_unlock_hotplug();
 	synchronize_rcu();
 out_free:
 	kfree(root);
@@ -643,7 +647,9 @@ static int acpi_pci_root_start(struct acpi_device *device)
 {
 	struct acpi_pci_root *root = acpi_driver_data(device);
 
+	pci_lock_hotplug();
 	pci_bus_add_devices(root->bus);
+	pci_unlock_hotplug();
 	return 0;
 }
 
@@ -652,6 +658,7 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
+	pci_lock_hotplug();
 	mutex_lock(&acpi_pci_root_lock);
 
 	list_for_each_entry(driver, &acpi_pci_drivers, node)
@@ -675,6 +682,7 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 out:
 	list_del_rcu(&root->node);
 	mutex_unlock(&acpi_pci_root_lock);
+	pci_unlock_hotplug();
 	synchronize_rcu();
 	kfree(root);
 
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [RFC PATCH 11/11] PCI: Serialize hotplug operations triggered by acpiphp driver
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
                   ` (9 preceding siblings ...)
  2012-03-23 14:58 ` [RFC PATCH 10/11] PCI,ACPI: serialize hotplug operations triggered by ACPI subsystem Jiang Liu
@ 2012-03-23 14:58 ` Jiang Liu
  2012-03-27  3:33 ` [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Kenji Kaneshige
  11 siblings, 0 replies; 15+ messages in thread
From: Jiang Liu @ 2012-03-23 14:58 UTC (permalink / raw)
  To: Yinghai Lu, Jesse Barnes, Len Brown
  Cc: Jiang Liu, Bjorn Helgaas, Ashok Raj, Jiang Liu, Keping Chen,
	linux-pci, linux-acpi

User pci_lock_hotplug()/pci_unlock_hotplug() to serialize PCI
hotplug operations triggered by acpiphp driver.

Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/pci/hotplug/acpiphp_glue.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/hotplug/acpiphp_glue.c b/drivers/pci/hotplug/acpiphp_glue.c
index 764891e..ee820a9 100644
--- a/drivers/pci/hotplug/acpiphp_glue.c
+++ b/drivers/pci/hotplug/acpiphp_glue.c
@@ -1095,6 +1095,8 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 	handle = hp_work->handle;
 	type = hp_work->type;
 	bridge = (struct acpiphp_bridge *)hp_work->context;
+
+	pci_lock_hotplug();
 	if (bridge->flags & BRIDGE_IS_DEAD)
 		goto out;
 
@@ -1156,6 +1158,7 @@ static void _handle_hotplug_event_bridge(struct work_struct *work)
 	}
 
 out:
+	pci_unlock_hotplug();
 	kref_put(&bridge->kref, acpiphp_release_bridge);
 	kfree(hp_work); /* allocated in handle_hotplug_event_bridge */
 }
@@ -1201,6 +1204,8 @@ static void _handle_hotplug_event_func(struct work_struct *work)
 	handle = hp_work->handle;
 	type = hp_work->type;
 	func = (struct acpiphp_func *)hp_work->context;
+
+	pci_lock_hotplug();
 	if (func->flags & FUNC_IS_DEAD)
 		goto out;
 
@@ -1237,6 +1242,7 @@ static void _handle_hotplug_event_func(struct work_struct *work)
 	}
 
 out:
+	pci_unlock_hotplug();
 	kref_put(&func->kref, acpiphp_release_func);
 	kfree(hp_work); /* allocated in handle_hotplug_event_func */
 }
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem
  2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
                   ` (10 preceding siblings ...)
  2012-03-23 14:58 ` [RFC PATCH 11/11] PCI: Serialize hotplug operations triggered by acpiphp driver Jiang Liu
@ 2012-03-27  3:33 ` Kenji Kaneshige
  2012-03-27 14:31   ` Jiang Liu
  11 siblings, 1 reply; 15+ messages in thread
From: Kenji Kaneshige @ 2012-03-27  3:33 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Yinghai Lu, Jesse Barnes, Len Brown, Jiang Liu, Bjorn Helgaas,
	Ashok Raj, Keping Chen, linux-pci, linux-acpi

> The second prososal is to introduce a recursive mutex lock to
> serialize all hotplug operations triggered by sysfs interfaces,
> PCI HPC hardware events, ACPI hotplug events and other sources.

The pciehp (and other hotplug controllers) needs this too.

There is only one mutex in the current implementation. I think it
would be better if we can have the mutex per host bridge or per bus
or something so that we can run multiple hot-plug operation in
parallel on the independent PCI sub trees.

Regards,
Kenji Kaneshige



(2012/03/23 23:58), Jiang Liu wrote:
> This is a series of RFC patches, to make sure we are doing the
> right thing in the right way.  The patchset hasn't been tested yet.
> There are several minor bugfixes and two proposals.
> 
> The first proposal is to add notification chain for PCI hotplug events,
> so other components interested in PCI root/bus/device hotplug events
> could subscribe to the chain.
> 
> The second prososal is to introduce a recursive mutex lock to
> serialize all hotplug operations triggered by sysfs interfaces,
> PCI HPC hardware events, ACPI hotplug events and other sources.
> 
> There are still works left in driver/pci/hotplug directory to apply
> the above two proposals.
> 
> The patchset applies to Yinghai's work on PCI root bus hotplug at
> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-irq
> And it depends on my previous patchset, please refer to
> http://www.spinics.net/lists/linux-pci/msg14472.html
> 
> Thanks!
> 
> Jiang Liu (11):
>    PCI: Fix device reference count leakage in pci_dev_present()
>    PCI: introduce pci_bus_get()/pci_bus_put() to hide implementation
>      details
>    PCI: clean up root bridge related logic in acpiphp driver
>    ACPI,PCI: fix race windows caused by alloc_acpi_hotplug_work()
>    PCI: Add notification interfaces for PCI root/bus/device hotplug
>      events
>    ACPI,PCI: update ACPI<->PCI binding information when pci hotplug
>      event happens
>    ACPI,PCI: update ACPI slots when PCI hotplug event happens
>    PCI: Introduce recursive mutex to serialize PCI hotplug operations
>    PCI: serialize hotplug operations triggered by PCI hotplug sysfs
>      interfaces
>    PCI,ACPI: serialize hotplug operations triggered by ACPI subsystem
>    PCI: Serialize hotplug operations triggered by acpiphp driver
> 
>   drivers/acpi/pci_bind.c                |  115 +++++++++++++++++++++++++
>   drivers/acpi/pci_root.c                |    8 ++
>   drivers/acpi/pci_slot.c                |   81 ++++++++++++++++++-
>   drivers/pci/bus.c                      |   20 ++++-
>   drivers/pci/hotplug.c                  |   47 +++++++++++
>   drivers/pci/hotplug/acpiphp.h          |    7 ++-
>   drivers/pci/hotplug/acpiphp_glue.c     |  143 ++++++++++++--------------------
>   drivers/pci/hotplug/pci_hotplug_core.c |    2 +
>   drivers/pci/pci-sysfs.c                |   29 +++----
>   drivers/pci/pci.h                      |    8 ++
>   drivers/pci/probe.c                    |   10 ++-
>   drivers/pci/remove.c                   |   20 ++++-
>   drivers/pci/search.c                   |   10 +-
>   include/linux/pci.h                    |   57 +++++++++++++
>   14 files changed, 437 insertions(+), 120 deletions(-)
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem
  2012-03-27  3:33 ` [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Kenji Kaneshige
@ 2012-03-27 14:31   ` Jiang Liu
  2012-03-30  4:15     ` Kenji Kaneshige
  0 siblings, 1 reply; 15+ messages in thread
From: Jiang Liu @ 2012-03-27 14:31 UTC (permalink / raw)
  To: Kenji Kaneshige
  Cc: Yinghai Lu, Jesse Barnes, Len Brown, Jiang Liu, Bjorn Helgaas,
	Ashok Raj, Keping Chen, linux-pci, linux-acpi

Hi Kenji,
	Thanks for your comments!
	As you have mentioned, I have a TODO list for all PCI hotplug drivers 
under drivers/pci/hotplug. I plan to modify those drivers after doing stress
tests against the acpiphp driver. But one difficulty I'm facing is to find 
hardware platforms to test those hotplug drivers, currently I could only find
platforms to test the acpiphp and pcie hotplug drivers.
	For the lock granularity issue,	I have thought about providing a more
fine-grained lock schema, such as "branch lock" which locks PCI sub trees. 
But that solution needs much more changes and careful inspection over
the PCI code to avoid deadlock issues. So I think it would be better for us
to globally serialize all PCI hotplug operations first and optimize the lock
schema in future. Is that OK to you?
	Thanks!
	Gerry

On 03/27/2012 11:33 AM, Kenji Kaneshige wrote:
>> The second prososal is to introduce a recursive mutex lock to
>> serialize all hotplug operations triggered by sysfs interfaces,
>> PCI HPC hardware events, ACPI hotplug events and other sources.
> 
> The pciehp (and other hotplug controllers) needs this too.
> 
> There is only one mutex in the current implementation. I think it
> would be better if we can have the mutex per host bridge or per bus
> or something so that we can run multiple hot-plug operation in
> parallel on the independent PCI sub trees.
> 
> Regards,
> Kenji Kaneshige
> 
> 
> 
> (2012/03/23 23:58), Jiang Liu wrote:
>> This is a series of RFC patches, to make sure we are doing the
>> right thing in the right way.  The patchset hasn't been tested yet.
>> There are several minor bugfixes and two proposals.
>>
>> The first proposal is to add notification chain for PCI hotplug events,
>> so other components interested in PCI root/bus/device hotplug events
>> could subscribe to the chain.
>>
>> The second prososal is to introduce a recursive mutex lock to
>> serialize all hotplug operations triggered by sysfs interfaces,
>> PCI HPC hardware events, ACPI hotplug events and other sources.
>>
>> There are still works left in driver/pci/hotplug directory to apply
>> the above two proposals.
>>
>> The patchset applies to Yinghai's work on PCI root bus hotplug at
>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-irq
>> And it depends on my previous patchset, please refer to
>> http://www.spinics.net/lists/linux-pci/msg14472.html
>>
>> Thanks!
>>
>> Jiang Liu (11):
>>    PCI: Fix device reference count leakage in pci_dev_present()
>>    PCI: introduce pci_bus_get()/pci_bus_put() to hide implementation
>>      details
>>    PCI: clean up root bridge related logic in acpiphp driver
>>    ACPI,PCI: fix race windows caused by alloc_acpi_hotplug_work()
>>    PCI: Add notification interfaces for PCI root/bus/device hotplug
>>      events
>>    ACPI,PCI: update ACPI<->PCI binding information when pci hotplug
>>      event happens
>>    ACPI,PCI: update ACPI slots when PCI hotplug event happens
>>    PCI: Introduce recursive mutex to serialize PCI hotplug operations
>>    PCI: serialize hotplug operations triggered by PCI hotplug sysfs
>>      interfaces
>>    PCI,ACPI: serialize hotplug operations triggered by ACPI subsystem
>>    PCI: Serialize hotplug operations triggered by acpiphp driver
>>
>>   drivers/acpi/pci_bind.c                |  115 +++++++++++++++++++++++++
>>   drivers/acpi/pci_root.c                |    8 ++
>>   drivers/acpi/pci_slot.c                |   81 ++++++++++++++++++-
>>   drivers/pci/bus.c                      |   20 ++++-
>>   drivers/pci/hotplug.c                  |   47 +++++++++++
>>   drivers/pci/hotplug/acpiphp.h          |    7 ++-
>>   drivers/pci/hotplug/acpiphp_glue.c     |  143 ++++++++++++--------------------
>>   drivers/pci/hotplug/pci_hotplug_core.c |    2 +
>>   drivers/pci/pci-sysfs.c                |   29 +++----
>>   drivers/pci/pci.h                      |    8 ++
>>   drivers/pci/probe.c                    |   10 ++-
>>   drivers/pci/remove.c                   |   20 ++++-
>>   drivers/pci/search.c                   |   10 +-
>>   include/linux/pci.h                    |   57 +++++++++++++
>>   14 files changed, 437 insertions(+), 120 deletions(-)
>>
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem
  2012-03-27 14:31   ` Jiang Liu
@ 2012-03-30  4:15     ` Kenji Kaneshige
  0 siblings, 0 replies; 15+ messages in thread
From: Kenji Kaneshige @ 2012-03-30  4:15 UTC (permalink / raw)
  To: Jiang Liu
  Cc: Yinghai Lu, Jesse Barnes, Len Brown, Jiang Liu, Bjorn Helgaas,
	Ashok Raj, Keping Chen, linux-pci, linux-acpi

(2012/03/27 23:31), Jiang Liu wrote:
> Hi Kenji,
> 	Thanks for your comments!
> 	As you have mentioned, I have a TODO list for all PCI hotplug drivers
> under drivers/pci/hotplug. I plan to modify those drivers after doing stress
> tests against the acpiphp driver. But one difficulty I'm facing is to find
> hardware platforms to test those hotplug drivers, currently I could only find
> platforms to test the acpiphp and pcie hotplug drivers.

I might be able to access the platform to test the shpchp driver.

> 	For the lock granularity issue,	I have thought about providing a more
> fine-grained lock schema, such as "branch lock" which locks PCI sub trees.
> But that solution needs much more changes and careful inspection over
> the PCI code to avoid deadlock issues. So I think it would be better for us
> to globally serialize all PCI hotplug operations first and optimize the lock
> schema in future. Is that OK to you?

Yes, it's OK.

By the way, your lock mechanism is required even without host bridge hotplug.
So I think you should implement it against the latest PCI source tree.

Regards,
Kenji Kaneshige



> 	Thanks!
> 	Gerry
> 
> On 03/27/2012 11:33 AM, Kenji Kaneshige wrote:
>>> The second prososal is to introduce a recursive mutex lock to
>>> serialize all hotplug operations triggered by sysfs interfaces,
>>> PCI HPC hardware events, ACPI hotplug events and other sources.
>>
>> The pciehp (and other hotplug controllers) needs this too.
>>
>> There is only one mutex in the current implementation. I think it
>> would be better if we can have the mutex per host bridge or per bus
>> or something so that we can run multiple hot-plug operation in
>> parallel on the independent PCI sub trees.
>>
>> Regards,
>> Kenji Kaneshige
>>
>>
>>
>> (2012/03/23 23:58), Jiang Liu wrote:
>>> This is a series of RFC patches, to make sure we are doing the
>>> right thing in the right way.  The patchset hasn't been tested yet.
>>> There are several minor bugfixes and two proposals.
>>>
>>> The first proposal is to add notification chain for PCI hotplug events,
>>> so other components interested in PCI root/bus/device hotplug events
>>> could subscribe to the chain.
>>>
>>> The second prososal is to introduce a recursive mutex lock to
>>> serialize all hotplug operations triggered by sysfs interfaces,
>>> PCI HPC hardware events, ACPI hotplug events and other sources.
>>>
>>> There are still works left in driver/pci/hotplug directory to apply
>>> the above two proposals.
>>>
>>> The patchset applies to Yinghai's work on PCI root bus hotplug at
>>> git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-x86-irq
>>> And it depends on my previous patchset, please refer to
>>> http://www.spinics.net/lists/linux-pci/msg14472.html
>>>
>>> Thanks!
>>>
>>> Jiang Liu (11):
>>>     PCI: Fix device reference count leakage in pci_dev_present()
>>>     PCI: introduce pci_bus_get()/pci_bus_put() to hide implementation
>>>       details
>>>     PCI: clean up root bridge related logic in acpiphp driver
>>>     ACPI,PCI: fix race windows caused by alloc_acpi_hotplug_work()
>>>     PCI: Add notification interfaces for PCI root/bus/device hotplug
>>>       events
>>>     ACPI,PCI: update ACPI<->PCI binding information when pci hotplug
>>>       event happens
>>>     ACPI,PCI: update ACPI slots when PCI hotplug event happens
>>>     PCI: Introduce recursive mutex to serialize PCI hotplug operations
>>>     PCI: serialize hotplug operations triggered by PCI hotplug sysfs
>>>       interfaces
>>>     PCI,ACPI: serialize hotplug operations triggered by ACPI subsystem
>>>     PCI: Serialize hotplug operations triggered by acpiphp driver
>>>
>>>    drivers/acpi/pci_bind.c                |  115 +++++++++++++++++++++++++
>>>    drivers/acpi/pci_root.c                |    8 ++
>>>    drivers/acpi/pci_slot.c                |   81 ++++++++++++++++++-
>>>    drivers/pci/bus.c                      |   20 ++++-
>>>    drivers/pci/hotplug.c                  |   47 +++++++++++
>>>    drivers/pci/hotplug/acpiphp.h          |    7 ++-
>>>    drivers/pci/hotplug/acpiphp_glue.c     |  143 ++++++++++++--------------------
>>>    drivers/pci/hotplug/pci_hotplug_core.c |    2 +
>>>    drivers/pci/pci-sysfs.c                |   29 +++----
>>>    drivers/pci/pci.h                      |    8 ++
>>>    drivers/pci/probe.c                    |   10 ++-
>>>    drivers/pci/remove.c                   |   20 ++++-
>>>    drivers/pci/search.c                   |   10 +-
>>>    include/linux/pci.h                    |   57 +++++++++++++
>>>    14 files changed, 437 insertions(+), 120 deletions(-)
>>>
>>
> 
> --
> 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
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-03-30  4:15 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-23 14:58 [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Jiang Liu
2012-03-23 14:58 ` [PATCH 01/11] PCI: Fix device reference count leakage in pci_dev_present() Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 02/11] PCI: introduce pci_bus_get()/pci_bus_put() to hide implementation details Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 03/11] PCI: clean up root bridge related logic in acpiphp driver Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 04/11] ACPI,PCI: fix race windows caused by alloc_acpi_hotplug_work() Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 05/11] PCI: Add notification interfaces for PCI root/bus/device hotplug events Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 06/11] ACPI,PCI: update ACPI<->PCI binding information when pci hotplug event happens Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 07/11] ACPI,PCI: update ACPI slots when PCI " Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 08/11] PCI: Introduce recursive mutex to serialize PCI hotplug operations Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 09/11] PCI: serialize hotplug operations triggered by PCI hotplug sysfs interfaces Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 10/11] PCI,ACPI: serialize hotplug operations triggered by ACPI subsystem Jiang Liu
2012-03-23 14:58 ` [RFC PATCH 11/11] PCI: Serialize hotplug operations triggered by acpiphp driver Jiang Liu
2012-03-27  3:33 ` [PATCH 00/11] Enhancements and bugfixes to PCI hotplug subsystem Kenji Kaneshige
2012-03-27 14:31   ` Jiang Liu
2012-03-30  4:15     ` Kenji Kaneshige

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.