Patchwork [RFC,v1,07/22] PCI: introduce PCI bus lock to serialize PCI hotplug operations

login
register
mail settings
Submitter Jiang Liu
Date Aug. 7, 2012, 4:10 p.m.
Message ID <1344355862-2726-8-git-send-email-jiang.liu@huawei.com>
Download mbox | patch
Permalink /patch/175717/
State Not Applicable
Headers show

Comments

Jiang Liu - Aug. 7, 2012, 4:10 p.m.
There are multiple ways to trigger concurrent PCI hotplug operations for
a specific PCI bus, but we have no way to serialize those PCI hotplug
operations yet and thus breaks the PCI hotplug logic. This patch introduces
a bus lock mechanism and state machine for PCI buses to serialize PCI
hotplug operations.

The state machine for PCI buses is:
          __________________________     ______________
          |                        v     |            v
INITIALIZED->REGISTERED->WORKING->STOPPING->STOPPED->REMOVED->DESTOYED
                     |_________________________^

The PCI buses is hierarchy, so need to obey the locking rules:
1) The PCI bus must be locked when changing any child devices of it.
2) The PCI bus must be locked when changing its state
3) The global PCI host bridge hotplug lock must be held when hotplugging
   PCI root buses

The lock interfaces cordinated with the state machine will be used to
avoid race conditions when hotplugging PCI devices/host bridges.
A typical usage is (lock bus if it's in WORKING state, and then do hotplug):
if (pci_bus_lock_states(bus, PCI_BUS_STATE_WORKING) > 0) {
	do_pci_hotplug();
	pci_bus_unlock(bus);
}

The PCI_BUS_LOCK config option is a temporary solution to avoid breaking
bisect, it will be removed when all Archs have been converted to the new
PCI bus lock mechanism.

Signed-off-by: Jiang Liu <liuj97@gmail.com>
---
 drivers/pci/Kconfig |    4 +++
 drivers/pci/bus.c   |   86 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |   44 ++++++++++++++++++++++++++
 3 files changed, 134 insertions(+)
Bjorn Helgaas - Sept. 11, 2012, 11:24 p.m.
On Tue, Aug 7, 2012 at 10:10 AM, Jiang Liu <liuj97@gmail.com> wrote:
> There are multiple ways to trigger concurrent PCI hotplug operations for
> a specific PCI bus, but we have no way to serialize those PCI hotplug
> operations yet and thus breaks the PCI hotplug logic. This patch introduces
> a bus lock mechanism and state machine for PCI buses to serialize PCI
> hotplug operations.
>
> The state machine for PCI buses is:
>           __________________________     ______________
>           |                        v     |            v
> INITIALIZED->REGISTERED->WORKING->STOPPING->STOPPED->REMOVED->DESTOYED
>                      |_________________________^
>
> The PCI buses is hierarchy, so need to obey the locking rules:
> 1) The PCI bus must be locked when changing any child devices of it.
> 2) The PCI bus must be locked when changing its state
> 3) The global PCI host bridge hotplug lock must be held when hotplugging
>    PCI root buses
>
> The lock interfaces cordinated with the state machine will be used to
> avoid race conditions when hotplugging PCI devices/host bridges.
> A typical usage is (lock bus if it's in WORKING state, and then do hotplug):
> if (pci_bus_lock_states(bus, PCI_BUS_STATE_WORKING) > 0) {
>         do_pci_hotplug();
>         pci_bus_unlock(bus);
> }
>
> The PCI_BUS_LOCK config option is a temporary solution to avoid breaking
> bisect, it will be removed when all Archs have been converted to the new
> PCI bus lock mechanism.

I'm going to wait until I understand the global lock issues better
before I even look at these PCI bus lock patches.

> Signed-off-by: Jiang Liu <liuj97@gmail.com>
> ---
>  drivers/pci/Kconfig |    4 +++
>  drivers/pci/bus.c   |   86 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |   44 ++++++++++++++++++++++++++
>  3 files changed, 134 insertions(+)
>
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 848bfb8..a6df8b1 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -120,3 +120,7 @@ config PCI_IOAPIC
>  config PCI_LABEL
>         def_bool y if (DMI || ACPI)
>         select NLS
> +
> +config PCI_BUS_LOCK
> +       bool
> +       default n
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 0e18270..aa25fcf 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -15,9 +15,12 @@
>  #include <linux/proc_fs.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> +#include <linux/sched.h>
>
>  #include "pci.h"
>
> +static DECLARE_WAIT_QUEUE_HEAD(pci_bus_state_wait_queue);
> +
>  void pci_add_resource_offset(struct list_head *resources, struct resource *res,
>                              resource_size_t offset)
>  {
> @@ -340,6 +343,89 @@ void pci_bus_put(struct pci_bus *bus)
>  }
>  EXPORT_SYMBOL(pci_bus_put);
>
> +static bool pci_bus_wait_for_states(struct pci_bus *bus, int states)
> +{
> +       int t = atomic_read(&bus->state);
> +
> +       /* Bus state is bigger than any of the requested states. */
> +       if ((t & PCI_BUS_STATE_MASK) > states)
> +               return true;
> +
> +       /* Bus is in one of the requested states and unlocked. */
> +       if ((t & states) && !(t & PCI_BUS_STATE_LOCK))
> +               return true;
> +
> +       return false;
> +}
> +
> +/*
> + * Wait for the bus to reach one of the requested states and then lock it.
> + * Return current bus state if succeed to lock the bus, and return -EINVAL
> + * if current bus state is already bigger than any of the requested states.
> + */
> +int pci_bus_lock_states(struct pci_bus *bus, int states)
> +{
> +       int t;
> +
> +       BUG_ON(states & ~PCI_BUS_STATE_MASK);
> +       do {
> +               do {
> +                       wait_event(pci_bus_state_wait_queue,
> +                                  pci_bus_wait_for_states(bus, states));
> +                       t = atomic_read(&bus->state);
> +                       if ((t & PCI_BUS_STATE_MASK) > states)
> +                               return -EINVAL;
> +               } while (!(t & states));
> +
> +               t &= ~PCI_BUS_STATE_LOCK;
> +       } while (atomic_cmpxchg(&bus->state, t , t | PCI_BUS_STATE_LOCK) != t);
> +
> +       return t & PCI_BUS_STATE_MASK;
> +}
> +EXPORT_SYMBOL(pci_bus_lock_states);
> +
> +/* Unlock the bus and wake up waiters, must be called with the bus locked. */
> +void pci_bus_unlock(struct pci_bus *bus)
> +{
> +       int t;
> +
> +       BUG_ON(!pci_bus_is_locked(bus));
> +       do {
> +               t = atomic_read(&bus->state);
> +       } while (atomic_cmpxchg(&bus->state,
> +                               t, t & ~PCI_BUS_STATE_LOCK) != t);
> +
> +       if (waitqueue_active(&pci_bus_state_wait_queue))
> +               wake_up_all(&pci_bus_state_wait_queue);
> +}
> +EXPORT_SYMBOL(pci_bus_unlock);
> +
> +/*
> + * Change the bus from old state to new state. It must be called with the bus
> + * locked, and the new state must be bigger than the old state.
> + */
> +void pci_bus_change_state(struct pci_bus *bus, int old, int new, bool unlock)
> +{
> +       int t;
> +
> +       BUG_ON(!pci_bus_is_locked(bus));
> +       BUG_ON(new < old || pci_bus_get_state(bus) != old ||
> +              (new & ~PCI_BUS_STATE_MASK));
> +
> +       old |= PCI_BUS_STATE_LOCK;
> +       if (!unlock)
> +               new |= PCI_BUS_STATE_LOCK;
> +
> +       do {
> +               t = atomic_read(&bus->state);
> +               t &= ~(PCI_BUS_STATE_MASK | PCI_BUS_STATE_LOCK);
> +       } while (atomic_cmpxchg(&bus->state, t | old, t | new) != (t | old));
> +
> +       if (waitqueue_active(&pci_bus_state_wait_queue))
> +               wake_up_all(&pci_bus_state_wait_queue);
> +}
> +EXPORT_SYMBOL(pci_bus_change_state);
> +
>  EXPORT_SYMBOL(pci_bus_alloc_resource);
>  EXPORT_SYMBOL_GPL(pci_bus_add_device);
>  EXPORT_SYMBOL(pci_bus_add_devices);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e02f130..e2ef517 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -443,8 +443,52 @@ struct pci_bus {
>         struct bin_attribute    *legacy_io; /* legacy I/O for this bus */
>         struct bin_attribute    *legacy_mem; /* legacy mem */
>         unsigned int            is_added:1;
> +       atomic_t                state;
>  };
>
> +/*
> + * State machine for PCI buses.
> + *          __________________________     ______________
> + *          |                        v     |            v
> + * INITIALIZED->REGISTERED->WORKING->STOPPING->STOPPED->REMOVED->DESTOYED
> + *                     |_________________________^
> + */
> +#define        PCI_BUS_STATE_UNKNOWN           0x0     /* invalid state */
> +#define        PCI_BUS_STATE_INITIALIZED       0x1     /* device_initialize called */
> +#define        PCI_BUS_STATE_REGISTERED        0x2     /* device_add called */
> +#define        PCI_BUS_STATE_WORKING           0x4     /* working state */
> +#define        PCI_BUS_STATE_STOPPING          0x8     /* stopping devices */
> +#define        PCI_BUS_STATE_STOPPED           0x10    /* device_del called */
> +#define        PCI_BUS_STATE_REMOVED           0x20    /* bus deleted  */
> +#define        PCI_BUS_STATE_DESTROYED         0x40    /* invalid state */
> +#define        PCI_BUS_STATE_MASK              0x7F
> +
> +#ifdef CONFIG_PCI_BUS_LOCK
> +#define        PCI_BUS_STATE_LOCK              0x10000 /* for pci core only */
> +
> +static inline bool pci_bus_is_locked(struct pci_bus *bus)
> +{
> +       return !!(atomic_read(&bus->state) & PCI_BUS_STATE_LOCK);
> +}
> +#else /* CONFIG_PCI_BUS_LOCK */
> +#define        PCI_BUS_STATE_LOCK              0x0000  /* for pci core only */
> +
> +static inline bool pci_bus_is_locked(struct pci_bus *bus)
> +{
> +       return true;
> +}
> +#endif /* CONFIG_PCI_BUS_LOCK */
> +
> +static inline int pci_bus_get_state(struct pci_bus *bus)
> +{
> +       return atomic_read(&bus->state) & PCI_BUS_STATE_MASK;
> +}
> +
> +extern int pci_bus_lock_states(struct pci_bus *bus, int states);
> +extern void pci_bus_unlock(struct pci_bus *bus);
> +extern void pci_bus_change_state(struct pci_bus *bus, int new, int old,
> +                                bool unlock);
> +
>  #define pci_bus_b(n)   list_entry(n, struct pci_bus, node)
>  #define to_pci_bus(n)  container_of(n, struct pci_bus, dev)
>
> --
> 1.7.9.5
>
--
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

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
index 848bfb8..a6df8b1 100644
--- a/drivers/pci/Kconfig
+++ b/drivers/pci/Kconfig
@@ -120,3 +120,7 @@  config PCI_IOAPIC
 config PCI_LABEL
 	def_bool y if (DMI || ACPI)
 	select NLS
+
+config PCI_BUS_LOCK
+	bool
+	default n
diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 0e18270..aa25fcf 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -15,9 +15,12 @@ 
 #include <linux/proc_fs.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <linux/sched.h>
 
 #include "pci.h"
 
+static DECLARE_WAIT_QUEUE_HEAD(pci_bus_state_wait_queue);
+
 void pci_add_resource_offset(struct list_head *resources, struct resource *res,
 			     resource_size_t offset)
 {
@@ -340,6 +343,89 @@  void pci_bus_put(struct pci_bus *bus)
 }
 EXPORT_SYMBOL(pci_bus_put);
 
+static bool pci_bus_wait_for_states(struct pci_bus *bus, int states)
+{
+	int t = atomic_read(&bus->state);
+
+	/* Bus state is bigger than any of the requested states. */
+	if ((t & PCI_BUS_STATE_MASK) > states)
+		return true;
+
+	/* Bus is in one of the requested states and unlocked. */
+	if ((t & states) && !(t & PCI_BUS_STATE_LOCK))
+		return true;
+
+	return false;
+}
+
+/*
+ * Wait for the bus to reach one of the requested states and then lock it.
+ * Return current bus state if succeed to lock the bus, and return -EINVAL
+ * if current bus state is already bigger than any of the requested states.
+ */
+int pci_bus_lock_states(struct pci_bus *bus, int states)
+{
+	int t;
+
+	BUG_ON(states & ~PCI_BUS_STATE_MASK);
+	do {
+		do {
+			wait_event(pci_bus_state_wait_queue,
+				   pci_bus_wait_for_states(bus, states));
+			t = atomic_read(&bus->state);
+			if ((t & PCI_BUS_STATE_MASK) > states)
+				return -EINVAL;
+		} while (!(t & states));
+
+		t &= ~PCI_BUS_STATE_LOCK;
+	} while (atomic_cmpxchg(&bus->state, t , t | PCI_BUS_STATE_LOCK) != t);
+
+	return t & PCI_BUS_STATE_MASK;
+}
+EXPORT_SYMBOL(pci_bus_lock_states);
+
+/* Unlock the bus and wake up waiters, must be called with the bus locked. */
+void pci_bus_unlock(struct pci_bus *bus)
+{
+	int t;
+
+	BUG_ON(!pci_bus_is_locked(bus));
+	do {
+		t = atomic_read(&bus->state);
+	} while (atomic_cmpxchg(&bus->state,
+				t, t & ~PCI_BUS_STATE_LOCK) != t);
+
+	if (waitqueue_active(&pci_bus_state_wait_queue))
+		wake_up_all(&pci_bus_state_wait_queue);
+}
+EXPORT_SYMBOL(pci_bus_unlock);
+
+/*
+ * Change the bus from old state to new state. It must be called with the bus
+ * locked, and the new state must be bigger than the old state.
+ */
+void pci_bus_change_state(struct pci_bus *bus, int old, int new, bool unlock)
+{
+	int t;
+
+	BUG_ON(!pci_bus_is_locked(bus));
+	BUG_ON(new < old || pci_bus_get_state(bus) != old ||
+	       (new & ~PCI_BUS_STATE_MASK));
+
+	old |= PCI_BUS_STATE_LOCK;
+	if (!unlock)
+		new |= PCI_BUS_STATE_LOCK;
+
+	do {
+		t = atomic_read(&bus->state);
+		t &= ~(PCI_BUS_STATE_MASK | PCI_BUS_STATE_LOCK);
+	} while (atomic_cmpxchg(&bus->state, t | old, t | new) != (t | old));
+
+	if (waitqueue_active(&pci_bus_state_wait_queue))
+		wake_up_all(&pci_bus_state_wait_queue);
+}
+EXPORT_SYMBOL(pci_bus_change_state);
+
 EXPORT_SYMBOL(pci_bus_alloc_resource);
 EXPORT_SYMBOL_GPL(pci_bus_add_device);
 EXPORT_SYMBOL(pci_bus_add_devices);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e02f130..e2ef517 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -443,8 +443,52 @@  struct pci_bus {
 	struct bin_attribute	*legacy_io; /* legacy I/O for this bus */
 	struct bin_attribute	*legacy_mem; /* legacy mem */
 	unsigned int		is_added:1;
+	atomic_t		state;
 };
 
+/*
+ * State machine for PCI buses.
+ *          __________________________     ______________
+ *          |                        v     |            v
+ * INITIALIZED->REGISTERED->WORKING->STOPPING->STOPPED->REMOVED->DESTOYED
+ *                     |_________________________^
+ */
+#define	PCI_BUS_STATE_UNKNOWN		0x0	/* invalid state */
+#define	PCI_BUS_STATE_INITIALIZED	0x1	/* device_initialize called */
+#define	PCI_BUS_STATE_REGISTERED	0x2	/* device_add called */
+#define	PCI_BUS_STATE_WORKING		0x4	/* working state */
+#define	PCI_BUS_STATE_STOPPING		0x8	/* stopping devices */
+#define	PCI_BUS_STATE_STOPPED		0x10	/* device_del called */
+#define	PCI_BUS_STATE_REMOVED		0x20	/* bus deleted  */
+#define	PCI_BUS_STATE_DESTROYED		0x40	/* invalid state */
+#define	PCI_BUS_STATE_MASK		0x7F
+
+#ifdef	CONFIG_PCI_BUS_LOCK
+#define	PCI_BUS_STATE_LOCK		0x10000	/* for pci core only */
+
+static inline bool pci_bus_is_locked(struct pci_bus *bus)
+{
+	return !!(atomic_read(&bus->state) & PCI_BUS_STATE_LOCK);
+}
+#else /* CONFIG_PCI_BUS_LOCK */
+#define	PCI_BUS_STATE_LOCK		0x0000	/* for pci core only */
+
+static inline bool pci_bus_is_locked(struct pci_bus *bus)
+{
+	return true;
+}
+#endif /* CONFIG_PCI_BUS_LOCK */
+
+static inline int pci_bus_get_state(struct pci_bus *bus)
+{
+	return atomic_read(&bus->state) & PCI_BUS_STATE_MASK;
+}
+
+extern int pci_bus_lock_states(struct pci_bus *bus, int states);
+extern void pci_bus_unlock(struct pci_bus *bus);
+extern void pci_bus_change_state(struct pci_bus *bus, int new, int old,
+				 bool unlock);
+
 #define pci_bus_b(n)	list_entry(n, struct pci_bus, node)
 #define to_pci_bus(n)	container_of(n, struct pci_bus, dev)