On 7/2/24 05:39, Bjorn Helgaas wrote: > [+cc Alex] > > On Thu, Jun 27, 2024 at 09:56:02AM +0900, Damien Le Moal wrote: >> On 6/27/24 00:15, Bjorn Helgaas wrote: >>>>> Yes, I am talking about the PCI "Function Level Reset" >>>>> >>>>>> FLR and disk/controller suspend execution timing are unrelated. >>>>>> FLR can be triggered at any time through sysfs. So please give >>>>>> details here. Why is FLR done when the system is being >>>>>> suspended ? >>>>> >>>>> Yes, it is because FLR can be triggered at any time that we are >>>>> testing the reliability of executing FLR commands after >>>>> disk/controller suspended. >>>> >>>> "can be triggered" ? FLR is not a random asynchronous event. It >>>> is an action that is *issued* by a user with sys admin rights. >>>> And such users can do a lot of things that can break a machine... >>>> >>>> I fail to see the point of doing a function reset while the >>>> device is suspended. But granted, I guess the device should >>>> comeback up in such case, though I would like to hear what the >>>> PCI guys have to say about this. >>>> >>>> Bjorn, >>>> >>>> Is reseting a suspended PCI device something that should be/is >>>> supported ? >>> >>> I doubt it. The PCI core should be preserving all the generic PCI >>> state across suspend/resume. The driver should only need to >>> save/restore device-specific things the PCI core doesn't know about. >>> >>> A reset will clear out most state, and the driver doesn't know the >>> reset happened, so it will expect most device state to have been >>> preserved. >> >> That is what I suspected. However, checking the code, reset_store() in >> pci-sysfs.c does: >> >> pm_runtime_get_sync(dev); >> result = pci_reset_function(pdev); >> pm_runtime_put(dev); >> >> and pm_runtime_get_sync() calls __pm_runtime_resume() which will >> resume a suspended device. >> >> So while I still think it is not a good idea to reset a suspended >> device, things should still work as execpected and not cause any >> problem with the device state, right ? > > The reset will clear almost all state, including both the generic PCI > part that pci_reset_function() saves/restores *and* any > device-specific state the PCI core doesn't know about. > > That device-specific state isn't saved and restored anywhere in the > sysfs reset path, and the driver doesn't know this reset happened, so > I think all bets are off and we shouldn't expect the driver to work > afterwards. > > A user-space reset might make sense if there's no driver bound to the > device, but I don't think it does if there is a driver (except maybe a > trivial stub driver that doesn't actually operate the device). OK, makes sense. I amstill looking into this though because I did find a nasty issue: if the HBA is reset while all the drives connected to it are suspended (spun down), the drives are never woken up and the drive re-scan trigerred by the PCI reset fails with command timeouts. And even worse, I hit a deadlock when unloading the driver after that happens. All of that should not be happening: the HBA reset should simply result in either all drives coming back or the drives (scsi devices) being dropped and re-scan creating new ones. But that I think is not a PCI issue but rather a HBA driver issue, or a problem with libsas/scsi/libata power management. Thanks for the comments. -- Damien Le Moal Western Digital Research
