On Mon, 21 Jun 2021 08:01:35 -0500 Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > On Sat, Jun 19, 2021 at 07:29:20PM +0530, Amey Narkhede wrote: > > On 21/06/18 03:00PM, Bjorn Helgaas wrote: > > > On Tue, Jun 08, 2021 at 11:18:53AM +0530, Amey Narkhede wrote: > > > > + while ((name = strsep(&options, ",")) != NULL) { > > > > + if (sysfs_streq(name, "")) > > > > + continue; > > > > + > > > > + name = strim(name); > > > > + > > > > + for (i = 0; i < PCI_RESET_METHODS_NUM; i++) { > > > > + if (reset_methods[i] && > > > > + sysfs_streq(name, pci_reset_fn_methods[i].name)) { > > > > + reset_methods[i] = prio--; > > > > + break; > > > > + } > > > > + } > > > > + > > > > + if (i == PCI_RESET_METHODS_NUM) { > > > > + kfree(options); > > > > + return -EINVAL; > > > > + } > > > > + } > > > > + > > > > + if (reset_methods[0] && > > > > + reset_methods[0] != PCI_RESET_METHODS_NUM) > > > > + pci_warn(pdev, "Device specific reset disabled/de-prioritized by user"); > > > > > > Is there a specific reason for this warning? Is it just telling the > > > user that he might have shot himself in the foot? Not sure that's > > > necessary. > > > > > I think generally presence of device specific reset method means other > > methods are potentially broken. Is it okay to skip this? > > We might want a warning at reset-time if all the methods failed, > because that means we may leak state between users. Maybe we also > want one here, if *all* reset methods are disabled. I don't really > like special treatment of device-specific methods here because it > depends on the assumption that "device-specific means all other resets > are broken." That's hard to maintain. I'd say the device specific reset is special. The device itself can support a number of resets and they're theoretically all equivalent, it's a policy decision which to use. But the device specific reset is a software provided reset. Someone has specifically gone to the trouble to create a reset mechanism that is in some way better than the other methods. Not using that one by default sure feels like something worthy of leaving a breadcrumb in dmesg for debugging. Thanks, Alex