Re: [PATCH 02/13] powerpc/eeh: Add final message for successful recovery
On Fri, May 04, 2018 at 04:08:28PM +1000, Russell Currey wrote:
> On Fri, 2018-05-04 at 12:55 +1000, Michael Ellerman wrote:
> > Sam Bobroff writes:
> >
> > > Add a single log line at the end of successful EEH recovery, so
> > > that
> > > it's clear that event processing has finished.
> > >
> > > Signed-off-by: Sam Bobroff
> > > ---
> > > arch/powerpc/kernel/eeh_driver.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/arch/powerpc/kernel/eeh_driver.c
> > > b/arch/powerpc/kernel/eeh_driver.c
> > > index 56a60b9eb397..07e0a42035ce 100644
> > > --- a/arch/powerpc/kernel/eeh_driver.c
> > > +++ b/arch/powerpc/kernel/eeh_driver.c
> > > @@ -910,6 +910,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
> > > pr_info("EEH: Notify device driver to resume\n");
> > > eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
> > >
> > > + pr_info("EEH: Recovery successful.\n");
> > Is it possible for recovery for multiple devices to be interleaved?
> >
> > Should that message include the device?
>
> Pretty sure EEH will only process a single error at a time so this
> *should* always let you infer from context, but PHB and PE should
> probably be included anyway. It'd be cool to move pe_{err/warn/info}()
> out of powernv for messages like this.
While we only process a single "event" at a time, that event covers the
initial PE, or possibly it's parent or PHB, and any dependent PEs so I
thought it could be misleading to show only a single one for that
message -- I wanted something to indicate that the whole "event" had
finished (since that was otherwise not really clear to me). Does that
seem reasonable?
I'm sorry but I don't understand what you mean by moving some functions
out of powernv, could you explain more?
> - Russell
>
> >
> > cheers
>
signature.asc
Description: PGP signature
Re: [PATCH 02/13] powerpc/eeh: Add final message for successful recovery
On Fri, May 04, 2018 at 12:55:42PM +1000, Michael Ellerman wrote:
> Sam Bobroff writes:
>
> > Add a single log line at the end of successful EEH recovery, so that
> > it's clear that event processing has finished.
> >
> > Signed-off-by: Sam Bobroff
> > ---
> > arch/powerpc/kernel/eeh_driver.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/powerpc/kernel/eeh_driver.c
> > b/arch/powerpc/kernel/eeh_driver.c
> > index 56a60b9eb397..07e0a42035ce 100644
> > --- a/arch/powerpc/kernel/eeh_driver.c
> > +++ b/arch/powerpc/kernel/eeh_driver.c
> > @@ -910,6 +910,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
> > pr_info("EEH: Notify device driver to resume\n");
> > eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
> >
> > + pr_info("EEH: Recovery successful.\n");
>
> Is it possible for recovery for multiple devices to be interleaved?
>
> Should that message include the device?
No, all the calls are serialized. They're only called from
eeh_event_handler() which is in the main loop of a kernel thread (see
eeh_event_init()).
> cheers
>
signature.asc
Description: PGP signature
Re: [PATCH 02/13] powerpc/eeh: Add final message for successful recovery
On Fri, 2018-05-04 at 12:55 +1000, Michael Ellerman wrote:
> Sam Bobroff writes:
>
> > Add a single log line at the end of successful EEH recovery, so
> > that
> > it's clear that event processing has finished.
> >
> > Signed-off-by: Sam Bobroff
> > ---
> > arch/powerpc/kernel/eeh_driver.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/powerpc/kernel/eeh_driver.c
> > b/arch/powerpc/kernel/eeh_driver.c
> > index 56a60b9eb397..07e0a42035ce 100644
> > --- a/arch/powerpc/kernel/eeh_driver.c
> > +++ b/arch/powerpc/kernel/eeh_driver.c
> > @@ -910,6 +910,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
> > pr_info("EEH: Notify device driver to resume\n");
> > eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
> >
> > + pr_info("EEH: Recovery successful.\n");
> Is it possible for recovery for multiple devices to be interleaved?
>
> Should that message include the device?
Pretty sure EEH will only process a single error at a time so this
*should* always let you infer from context, but PHB and PE should
probably be included anyway. It'd be cool to move pe_{err/warn/info}()
out of powernv for messages like this.
- Russell
>
> cheers
Re: [PATCH 02/13] powerpc/eeh: Add final message for successful recovery
Sam Bobroff writes:
> Add a single log line at the end of successful EEH recovery, so that
> it's clear that event processing has finished.
>
> Signed-off-by: Sam Bobroff
> ---
> arch/powerpc/kernel/eeh_driver.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kernel/eeh_driver.c
> b/arch/powerpc/kernel/eeh_driver.c
> index 56a60b9eb397..07e0a42035ce 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -910,6 +910,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
> pr_info("EEH: Notify device driver to resume\n");
> eeh_pe_dev_traverse(pe, eeh_report_resume, NULL);
>
> + pr_info("EEH: Recovery successful.\n");
Is it possible for recovery for multiple devices to be interleaved?
Should that message include the device?
cheers
