Actually forgot to add Guenter. Also +rthirumal. On Fri, Jun 12, 2015 at 11:13 AM, Rajat Jain <rajatxjain@xxxxxxxxx> wrote: > [+Guenter] > > Looks good to me. Unfortunately, I do not have the hardware anymore to > test it :-( > > Guenter: Do you have it, or know some body who has and wants to test/use this? > > Thanks, > > Rajat > > > On Fri, Jun 12, 2015 at 1:20 AM, Yijing Wang <wangyijing@xxxxxxxxxx> wrote: >> rajatjain@xxxxxxxxxxx is not reachable now. >> >> So add CC: rajatxjain@xxxxxxxxx >> >> On 2015/6/11 19:12, Yijing Wang wrote: >>> Rajat Jain reported a deadlock when a hierarchical hot plug >>> thread and aer recovery thread both run. >>> https://lkml.org/lkml/2015/3/11/861 >>> >>> thread 1: >>> pciehp_enable_slot() >>> pciehp_configure_device() >>> pci_bus_add_devices() >>> device_attach(dev) >>> device_lock(dev) //acquire device mutex successfully >>> ... >>> pciehp_probe(dev) >>> __pci_hp_register() >>> pci_create_slot() >>> down_write(pci_bus_sem) //deadlock here >>> >>> thread 2: >>> aer_isr_one_error() >>> aer_process_err_device() >>> do_recovery() >>> broadcast_error_message() >>> pci_walk_bus() >>> down_read(&pci_bus_sem) //acquire pci_bus_sem successfully >>> report_error_detected(dev) >>> device_lock(dev) // deadlock here >>> >>> Now we use pci_bus_sem to protect pci_slot creation and destroy, >>> it's unnecessary. We could introduce a new local mutex instead of >>> pci_bus_sem to avoid the deadlock. >>> >>> Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx> >>> --- >>> drivers/pci/slot.c | 11 ++++++----- >>> 1 files changed, 6 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c >>> index 396c200..feb08de 100644 >>> --- a/drivers/pci/slot.c >>> +++ b/drivers/pci/slot.c >>> @@ -14,6 +14,7 @@ >>> >>> struct kset *pci_slots_kset; >>> EXPORT_SYMBOL_GPL(pci_slots_kset); >>> +static DEFINE_MUTEX(pci_slot_mutex); >>> >>> static ssize_t pci_slot_attr_show(struct kobject *kobj, >>> struct attribute *attr, char *buf) >>> @@ -195,7 +196,7 @@ static struct pci_slot *get_slot(struct pci_bus *parent, int slot_nr) >>> { >>> struct pci_slot *slot; >>> /* >>> - * We already hold pci_bus_sem so don't worry >>> + * We already hold pci_slot_mutex so don't worry >>> */ >>> list_for_each_entry(slot, &parent->slots, list) >>> if (slot->number == slot_nr) { >>> @@ -253,7 +254,7 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, >>> int err = 0; >>> char *slot_name = NULL; >>> >>> - down_write(&pci_bus_sem); >>> + mutex_lock(&pci_slot_mutex); >>> >>> if (slot_nr == -1) >>> goto placeholder; >>> @@ -310,7 +311,7 @@ placeholder: >>> >>> out: >>> kfree(slot_name); >>> - up_write(&pci_bus_sem); >>> + mutex_unlock(&pci_slot_mutex); >>> return slot; >>> err: >>> kfree(slot); >>> @@ -332,9 +333,9 @@ void pci_destroy_slot(struct pci_slot *slot) >>> dev_dbg(&slot->bus->dev, "dev %02x, dec refcount to %d\n", >>> slot->number, atomic_read(&slot->kobj.kref.refcount) - 1); >>> >>> - down_write(&pci_bus_sem); >>> + mutex_lock(&pci_slot_mutex); >>> kobject_put(&slot->kobj); >>> - up_write(&pci_bus_sem); >>> + mutex_unlock(&pci_slot_mutex); >>> } >>> EXPORT_SYMBOL_GPL(pci_destroy_slot); >>> >>> >> >> >> -- >> Thanks! >> Yijing >> -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
