On Mon, Nov 12, 2012 at 2:17 AM, Yijing Wang <wangyijing@xxxxxxxxxx> wrote:
> Currently, pciehp use global pciehp_wq to handle hotplug event from hardware.
> Hot remove path will be blocked if a hotplug slot connected a IO-BOX(composed of PCIe
> Switch and some slots which support hotplug). The hot removed work was queued into
> pciehp_wq. But in the hot-remove path, pciehp driver would flush pciehp_wq when
> the pcie port(support pciehp) was removed. In this case the hot-remove path blocked.
> This patch remove the global pciehp_wq and create a new workqueue for every slot to
> avoid above problem.
Can you include the actual call path that leads to the deadlock? I
assume it's related to a PCIe device removal that happens at about the
same time as the pcie_port_service_unregister(&hpdriver_portdrv)?
I just want to make sure that the new per-slot workqueue arrangement
doesn't lead to any case where the slot workqueue item depends on
something removed by the pcied_cleanup() as the module exits.
> --- a/drivers/pci/hotplug/pciehp_hpc.c
> +++ b/drivers/pci/hotplug/pciehp_hpc.c
> @@ -773,23 +773,32 @@ static void pcie_shutdown_notification(struct controller *ctrl)
> static int pcie_init_slot(struct controller *ctrl)
> {
> struct slot *slot;
> + char name[32];
>
> slot = kzalloc(sizeof(*slot), GFP_KERNEL);
> if (!slot)
> return -ENOMEM;
> +
> + snprintf(name, sizeof(name), "pciehp-%u", PSN(ctrl));
> + slot->wq = create_singlethread_workqueue(name);
You're using create_singlethread_workqueue() instead of the previous
alloc_workqueue(). This is a slight change in semantics since we
previously called "alloc_workqueue(name, 0, 0)" and
create_singlethread_workqueue() calls "alloc_workqueue(name,
WQ_UNBOUND | WQ_MEM_RECLAIM, 1)."
I don't know whether this is necessary in order to have per-slot
workqueues or not. If not, please split it into a separate patch to
make it more visible.
Bjorn
--
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
