Re: [RFC][PATCH] PCI, pciehp: make every slot have its own workqueue to avoid deadlock

[Yijing Wang <wangyijing@huawei.com>]

On 2012/11/7 18:47, Kaneshige, Kenji wrote:
> Hi Yijing,
> 
> I have some comments. Sorry for the delay.
> 
> <snip.>
> 

Hi Kaneshige,
   Thanks for your review and comments very much!

I will update this patch and using slot name instead of device name, it's really a good suggestion.
Also I will remove the redundant flush_workqueue().

Thanks!
Yijing.

>> --- a/drivers/pci/hotplug/pciehp_hpc.c
>> +++ b/drivers/pci/hotplug/pciehp_hpc.c
>> @@ -773,23 +773,33 @@ 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), "pciehpd_%s",
>> +		pci_name(ctrl->pcie->port));
> 
> The name of the threads seems to be cut off.
> 
> $ ps aux | grep pciehp
> root        125  0.0  0.0      0     0 ?        S<   18:54   0:00 [pciehpd_0000:00]
> root        126  0.0  0.0      0     0 ?        S<   18:54   0:00 [pciehpd_0000:00]
> root        127  0.0  0.0      0     0 ?        S<   18:54   0:00 [pciehpd_0000:0b]
> root        128  0.0  0.0      0     0 ?        S<   18:54   0:00 [pciehpd_0000:0b]
> root        129  0.0  0.0      0     0 ?        S<   18:54   0:00 [pciehpd_0000:0b]
> root        130  0.0  0.0      0     0 ?        S<   18:54   0:00 [pciehpd_0000:0b]
> root        131  0.0  0.0      0     0 ?        S<   18:55   0:00 [pciehpd_0000:1a]
> root        132  0.0  0.0      0     0 ?        S<   18:55   0:00 [pciehpd_0000:1a]
> root        133  0.0  0.0      0     0 ?        S<   18:55   0:00 [pciehpd_0000:1a]
> root        134  0.0  0.0      0     0 ?        S<   18:55   0:00 [pciehpd_0000:1a]
> 
> How about using slot name for the thread name?
> 
> <snip.>
> 
>>
>>  static void pcie_cleanup_slot(struct controller *ctrl)
>>  {
>>  	struct slot *slot = ctrl->slot;
>>  	cancel_delayed_work(&slot->work);
>> -	flush_workqueue(pciehp_wq);
>> +	flush_workqueue(slot->wq);
>> +	destroy_workqueue(slot->wq);
>>  	kfree(slot);
>>  }
> 
> I think flush_workqueue() call is no longer required because
> it is done in destroy_workqueue() code path.
> 
> Regards,
> Kenji Kaneshige
> 
> 
> .
> 


-- 
Thanks!
Yijing