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
--
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
