On Fri, May 17, 2013 at 3:42 PM, Joe Lawrence <joe.lawrence@xxxxxxxxxxx> wrote: > On Fri, 17 May 2013 09:29:06 -0600 > Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > >> [+cc linux-pci] >> >> On Wed, May 15, 2013 at 11:29 AM, Joe Lawrence >> <joe.lawrence@xxxxxxxxxxx> wrote: >> > From 84ac7a35ebd61e84d4254eae78bb967de17254c2 Mon Sep 17 00:00:00 >> > 2001 From: Joe Lawrence <joe.lawrence@xxxxxxxxxxx> >> > Date: Wed, 15 May 2013 12:52:31 -0400 >> > Subject: [PATCH] mpt2sas,mpt3sas: make watchdog instantiated device >> > removal safe >> > >> > Calling pci_stop_and_remove_bus_device from a SCSI LLD may introduce >> > device removal races with PCI callback functions. >> > >> > Simplify the mpt2sas watchdog mechanism by separating PCI device >> > removal from SCSI midlayer detachment. When the watchdog wishes to >> > remove a SCSI device from the topology, detach from the SCSI >> > midlayer, leaving the PCI device alone. Adjust various pci_driver >> > callbacks to account for a potentially SCSI detached PCI device. >> >> I don't know the details of the SCSI detachment, but this approach >> looks much cleaner to me. > > Thanks for commenting, Bjorn. I think this approach more closely > represents what this watchdog is trying to accomplish. > > Off list, Sreekanth from LSI tested and noticed a few issues with this > patch: > > - mpt2sas_base_stop_watchdog is called twice: The call from > mpt2sas_base_detach is safe, but now unnecessary (as a call was > added earlier up in the PCI driver callbacks to ensure that the > watchdog was out of the way.) This second invocation can be removed. > > - If the watchdog detects a bad IOC, the watchdog remains running: > The watchdog workqueue isn't cleaned up until > mpt2sas_base_stop_watchdog is called, so in the case that the > watchdog removes the device from SCSI topo, the workqueue will > remain unused until PCI .remove/.shutdown cleans it up. Perhaps a > single watchdog that iterates over all adapters would be simpler? > > Finally, if SCSI topo detachment is all that is interesting here, would > it make more sense to move the watchdog into the MPT "scsi" code? I > haven't looked at the code yet, but this might make an MPT fusion patch > easier (due to dependencies between its "scsi" and "base" modules). Hi Joe, I noticed this when looking through old email. I can't remember if anything happened with this. It seems like nice work; did it go anywhere? Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
