Re: [Patch v1 0/7] mpt3sas: Hot-Plug Surprise removal support on IOC.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



l


On Tue, Sep 4, 2018 at 3:12 PM, Lukas Wunner <lukas@xxxxxxxxx> wrote:
> On Tue, Sep 04, 2018 at 11:19:04AM +0530, Sreekanth Reddy wrote:
>> On Fri, Aug 31, 2018 at 2:25 PM, Lukas Wunner <lukas@xxxxxxxxx> wrote:
>> > * Just reading the vendor ID may not be sufficient to detect unplug,
>> >   it may also read as "all ones" if the link is down due to error
>> >   recovery by DPC.
>>
>> So, is their any other way to detect pci device unplug apart from
>> reading the vendor ID, I mean we have check any other flags, etc?
>
> Many scsi drivers call pci_channel_offline() to detect inaccessibility
> of the device due to a PCI error:
> https://elixir.bootlin.com/linux/v4.19-rc2/ident/pci_channel_offline
>
> A patch is pending such that surprise removal can also be queried
> with that same function:
> https://www.spinics.net/lists/linux-pci/msg75722.html

Lukas, thanks for pointing to this pci_dev_is_disconnected() API. So
we can use this API directly instead of reading the vendor Id and
checking for all one's once this patch get accepted?

>
>
>> > * I don't see why you need to poll for the device's removal from a
>> >   watchdog thread.  pciehp will invoke your driver's ->remove hook
>> >   once the device is gone.
>>
>> If we have some three to four PCI devices and all pci devices are hot
>> unplugged simultaneously, then we observed that driver's-remove hook
>> is called sequentially. So it takes some time to call fourth PCI
>> device driver's->remove hook. so during this time we want all the
>> outstanding commands to be gracefully terminated and hence we added
>> this watchdog thread to quickly detect the hba unplug and take
>> necessary steps such as gracefully terminate the outstanding IOs and
>> stop receiving further IOs on it. At later time when PCI subsystem
>> calls driver's-remove hook then driver can quickly release the
>> resources allocated for this unplugged device.
>
> pciehp does the following as soon as the surprise removal event is handled:
>
>         pci_dev_set_disconnected(dev, NULL);
>         if (pci_has_subordinate(dev))
>                 pci_walk_bus(dev->subordinate, pci_dev_set_disconnected, NULL);
>
> Thus, once the above-linked patch lands, you'll be able to detect
> surprise removal by calling pci_channel_offline() or checking
> pdev->error_state == pci_channel_io_perm_failure, obviating the
> need to poll for surprise removal from a kthread.
>
> There's another patch pending to move pci_walk_bus() out of the
> pci_lock_rescan_remove() so that it runs even earlier, without
> pointlessly waiting for a lock:
> https://www.spinics.net/lists/linux-pci/msg75438.html

Yes due to this pci_lock_rescan_remove() lock, we observed that
driver's->remove hook called sequentially even though multiple HBA
devices are hot unplugged simultaneously.

I have one more instance where still we need this poll kthread, i.e
during the device probe time we have some commands which has more
timeout value (e.g. 300 seconds), so if user has unplugged this device
just after sending this more time-out valued command then driver has
to wait until this time-out value expires. i.e. this device is still
visible in lspci output until this 300 seconds timeout value expires
even though device is unplugged. if we have a poll kthread (which will
poll for every one second) then driver can early detect the unplugged
state and can terminate the outstanding commands and hence probe
operation can be completed quickly.


Also whether we need to wait for below patches get accepted? so that
we can post the new set of patches accommodating according to your
suggestions,
https://www.spinics.net/lists/linux-pci/msg75722.html
https://www.spinics.net/lists/linux-pci/msg75438.html

>
> Thanks,
>
> Lukas



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux