Re: [RESEND PATCH] PM: wakeup: Wakeup accounting for interrupts

[Loic Poulain <loic.poulain@linaro.org>]

Hi Rafael,

On Wed, 26 Jan 2022 at 19:23, Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Mon, Jan 24, 2022 at 10:14 AM Loic Poulain <loic.poulain@linaro.org> wrote:
> >
> > Most of the time, system wakeup is caused by a wakeup-enabled
> > interrupt, but the accounting is not done for the related wakeup
> > source, causing 'wrong' values reported by device's wakeup attributes
> > and debugfs stats (debug/wakeup_sources).
> >
> > This change reports a wakeup event for any wakeup-sources the irq is
> > attached with.
> >
> > Note: This only works for drivers explicitly attaching the irq to
> > a given device (e.g. with dev_pm_set_wake_irq).
> >
> > Note2: Some drivers call pm_wakeup_event() in their irq handler, but
> > not all, moreover, an interrupt can be disabled while being flagged
> > as wakeup source, and so accounting must be performed. This solution
> > ensures that accounting will always be done for the interrupt waking
> > up the host.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > ---
> >  drivers/base/power/wakeup.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> > index 99bda0da..2d75e057 100644
> > --- a/drivers/base/power/wakeup.c
> > +++ b/drivers/base/power/wakeup.c
> > @@ -952,8 +952,19 @@ void pm_wakeup_clear(bool reset)
> >  void pm_system_irq_wakeup(unsigned int irq_number)
> >  {
> >         if (pm_wakeup_irq == 0) {
> > +               struct wakeup_source *ws;
> > +               int srcuidx;
> > +
> >                 pm_wakeup_irq = irq_number;
> >                 pm_system_wakeup();
> > +
> > +               /* wakeup accounting */
> > +               srcuidx = srcu_read_lock(&wakeup_srcu);
>
> This is called from interrupt context, so srcu_read_lock() cannot be used here.


AFAIU from srcu.h, it is not prohibited, right? and we are not
blocking while locked here.

Regards,
Loic

>
>
> > +               list_for_each_entry_rcu(ws, &wakeup_sources, entry) {
> > +                       if (ws->wakeirq && ws->wakeirq->irq == irq_number)
> > +                               pm_wakeup_ws_event(ws, 0, false);
> > +               }
> > +               srcu_read_unlock(&wakeup_srcu, srcuidx);
> >         }
> >  }
> >
> > --
> > 2.7.4
> >