On 2012/11/12 16:51, Kaneshige, Kenji wrote: >> -----Original Message----- >> From: Yijing Wang [mailto:wangyijing@xxxxxxxxxx] >> Sent: Monday, November 12, 2012 5:28 PM >> To: Kaneshige, Kenji/金重 憲治 >> Cc: Bjorn Helgaas; Yinghai Lu; Rafael; Rusty Russell; Mauro Carvalho Chehab; Oliver Neukum; jiang.liu@xxxxxxxxxx; >> linux-pci@xxxxxxxxxxxxxxx; Hanjun Guo >> Subject: Re: [PATCH -v2] PCI, pciehp: make every slot have its own workqueue to avoid deadlock >> >> On 2012/11/12 15:17, Kaneshige, Kenji wrote: >>>> @@ -94,6 +93,8 @@ static int init_slot(struct controller *ctrl) >>>> struct hotplug_slot_info *info = NULL; >>>> struct hotplug_slot_ops *ops = NULL; >>>> char name[SLOT_NAME_SIZE]; >>>> + char *buffer; >>>> + int len; >>>> int retval = -ENOMEM; >>>> >>>> hotplug = kzalloc(sizeof(*hotplug), GFP_KERNEL); >>>> @@ -135,6 +136,19 @@ static int init_slot(struct controller *ctrl) >>>> if (retval) >>>> ctrl_err(ctrl, >>>> "pci_hp_register failed with error %d\n", retval); >>> >>> I think it's natural to go to out: here if retval != 0. >>> I guess you intentionally didn't do that because you might want to do >>> workqueue cleanup in pciehp_release_ctrl() code path. But it is confusing >>> and hard to understand. >> >> Hi Kaneshige, >> >> You are right, go to out here is better. > > Just in case, my proposal is the creating workqueue in pcie_init_slot() > as you did in your first patch. ok, i will create workqueue in pcie_init_slot() and use pciehp-%u format as workqueue name. I will send out the next version patch later. Thanks! Yijing. > >> >>> In the previous patch, you created the work queue in pcie_init_slot(). >>> I think it's better. Maybe the reason you moved it from pcie_init_slot() >>> to init_slot() was that you needed the slot name by pci_hp_register(). >>> >>> How about using physical slot number, which is same as pci slot name by >>> pci_hp_register() on the normal platform, for the workqueue name instead? >>> >> >> I think use physical slot number is ok, so what about workqueue name like slot(physical slot number) format ? >> >> root 45808 0.0 0.0 0 0 ? S< 16:25 0:00 [slot(0)] > > I think we need "pciehp" in the name. I prefer "pciehp-%u". > > Regards, > Kenji Kaneshige > > > . > -- 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
