On Sat, Jun 02, 2018 at 01:40:46AM +1000, Michael Ellerman wrote: > Sam Bobroff <[email protected]> writes: > > > As EEH event handling progresses, a cumulative result of type > > pci_ers_result is built up by (some of) the eeh_report_*() functions > > using either: > > if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc; > > if (*res == PCI_ERS_RESULT_NONE) *res = rc; > > or: > > if ((*res == PCI_ERS_RESULT_NONE) || > > (*res == PCI_ERS_RESULT_RECOVERED)) *res = rc; > > if (*res == PCI_ERS_RESULT_DISCONNECT && > > rc == PCI_ERS_RESULT_NEED_RESET) *res = rc; > > (Where *res is the accumulator.) > > > > However, the intent is not immediately clear and the result in some > > situations is order dependent. > > > > Address this by assigning a priority to each result value, and always > > merging to the highest priority. This renders the intent clear, and > > provides a stable value for all orderings. > > > > Signed-off-by: Sam Bobroff <[email protected]> > > --- > > ====== v1 -> v2: ====== > > > > * Added the value, and missing newline, to some WARN()s. > > * Improved name of merge_result() to pci_ers_merge_result(). > > * Adjusted the result priorities so that unknown doesn't overlap with _NONE. > > These === markers seem to have confused patchwork, they ended up in the > patch, and then git put them in the changelog. > > http://patchwork.ozlabs.org/patch/920194/ > > The usual format is just something like: > > v2 - Added the value, and missing newline, to some WARN()s. > - Improved name of merge_result() to pci_ers_merge_result(). > - Adjusted the result priorities so that unknown doesn't overlap with > _NONE. > > cheers
Oh! I'll change it!
Sam.
>
> > arch/powerpc/kernel/eeh_driver.c | 36 ++++++++++++++++++++++++++----------
> > 1 file changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/eeh_driver.c
> > b/arch/powerpc/kernel/eeh_driver.c
> > index 188d15c4fe3a..2d3cac584899 100644
> > --- a/arch/powerpc/kernel/eeh_driver.c
> > +++ b/arch/powerpc/kernel/eeh_driver.c
> > @@ -39,6 +39,29 @@ struct eeh_rmv_data {
> > int removed;
> > };
> >
> > +static int eeh_result_priority(enum pci_ers_result result)
> > +{
> > + switch (result) {
> > + case PCI_ERS_RESULT_NONE: return 1;
> > + case PCI_ERS_RESULT_NO_AER_DRIVER: return 2;
> > + case PCI_ERS_RESULT_RECOVERED: return 3;
> > + case PCI_ERS_RESULT_CAN_RECOVER: return 4;
> > + case PCI_ERS_RESULT_DISCONNECT: return 5;
> > + case PCI_ERS_RESULT_NEED_RESET: return 6;
> > + default:
> > + WARN_ONCE(1, "Unknown pci_ers_result value: %d\n", (int)result);
> > + return 0;
> > + }
> > +};
> > +
> > +static enum pci_ers_result pci_ers_merge_result(enum pci_ers_result old,
> > + enum pci_ers_result new)
> > +{
> > + if (eeh_result_priority(new) > eeh_result_priority(old))
> > + return new;
> > + return old;
> > +}
> > +
> > /**
> > * eeh_pcid_get - Get the PCI device driver
> > * @pdev: PCI device
> > @@ -206,9 +229,7 @@ static void *eeh_report_error(struct eeh_dev *edev,
> > void *userdata)
> >
> > rc = driver->err_handler->error_detected(dev, pci_channel_io_frozen);
> >
> > - /* A driver that needs a reset trumps all others */
> > - if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
> > - if (*res == PCI_ERS_RESULT_NONE) *res = rc;
> > + *res = pci_ers_merge_result(*res, rc);
> >
> > edev->in_error = true;
> > pci_uevent_ers(dev, PCI_ERS_RESULT_NONE);
> > @@ -249,9 +270,7 @@ static void *eeh_report_mmio_enabled(struct eeh_dev
> > *edev, void *userdata)
> >
> > rc = driver->err_handler->mmio_enabled(dev);
> >
> > - /* A driver that needs a reset trumps all others */
> > - if (rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
> > - if (*res == PCI_ERS_RESULT_NONE) *res = rc;
> > + *res = pci_ers_merge_result(*res, rc);
> >
> > out:
> > eeh_pcid_put(dev);
> > @@ -294,10 +313,7 @@ static void *eeh_report_reset(struct eeh_dev *edev,
> > void *userdata)
> > goto out;
> >
> > rc = driver->err_handler->slot_reset(dev);
> > - if ((*res == PCI_ERS_RESULT_NONE) ||
> > - (*res == PCI_ERS_RESULT_RECOVERED)) *res = rc;
> > - if (*res == PCI_ERS_RESULT_DISCONNECT &&
> > - rc == PCI_ERS_RESULT_NEED_RESET) *res = rc;
> > + *res = pci_ers_merge_result(*res, rc);
> >
> > out:
> > eeh_pcid_put(dev);
> > --
> > 2.16.1.74.g9b0b1f47b
>
signature.asc
Description: PGP signature
