On Thu, Sep 27, 2018 at 05:56:25PM -0500, Bjorn Helgaas wrote: > On Wed, Sep 26, 2018 at 04:19:25PM -0600, Keith Busch wrote: > > On Wed, Sep 26, 2018 at 05:01:16PM -0500, Bjorn Helgaas wrote: > > > On Thu, Sep 20, 2018 at 10:27:13AM -0600, Keith Busch wrote: > > > > The link reset always used the first bridge device, but AER broadcast > > > > error handling may have reported an end device. This means the reset may > > > > hit devices that were never notified of the impending error recovery. > > > > > > > > This patch uses the first downstream port in the hierarchy considered > > > > reliable. An error detected by a switch upstream port should mean it > > > > occurred on its upstream link, so the patch selects the parent device > > > > if the error is not a root or downstream port. > > > > > > I'm not really clear on what "Always use the first downstream port" > > > means. Always use it for *what*? > > And I forgot to ask what "first downstream port" means. The "first downstream port" was supposed to mean the first DSP we find when walking toward the root from the pci_dev that reported ERR_[NON]FATAL. By "use", I mean "walking down the sub-tree for error handling". After thinking more on this, that doesn't really capture the intent. A better way might be: Run error handling starting from the downstream port of the bus reporting an error I'm struggling to make that short enough for a changelog subject. > > I'll see if I can better rephrase. > > > > Error handling should notify all affected pci functions. If an end device > > detects and emits ERR_FATAL, the old way would have only notified that > > end-device driver, but other functions may be on or below the same bus. > > > > Using the downstream port that connects to that bus where the error was > > detectedas the anchor point to broadcast error handling progression, > > we can notify all functions so they have a chance to prepare for the > > link reset. > > So do I understand correctly that if the ERR_FATAL source is: > > - a Switch Upstream Port, you assume the problem is with the Link > upstream from the Port, and that Link may need to be reset, so you > notify everything below that Link, including the Upstream Port, > everything below it (the Downstream Ports and anything below > them), and potentially even any peers of the Upstream Port (is it > even possible for a Upstream Port to have peer multi-function > devices?) Yep, the Microsemi Switchtec is one such real life example of an end device function on the same bus as a USP. > - a Switch Downstream Port, you assume the Port (and the Link going > downstream) may need to be reset, so you notify the Port and > anything below it > > - an Endpoint, you assume the Link leading to the Endpoint may need > to be reset, so you notify the Endpoint, any peer multi-function > devices, any related SR-IOV devices, and any devices below a peer > that happens to be a bridge > > And this is different from before because it notifies more devices in > some cases? There was a pci_walk_bus() in broadcast_error_message(), > so we should have notified several devices in *some* cases, at least. broadcast_error_message() had been using the pci_dev that detected the error, and it's pci_walk_bus() used dev->subordinate. If the pci_dev that detected an error was an end device, we didn't walk the bus.
