[...]
> > > > > +static irqreturn_t __handle_irq_event_percpu(unsigned int irq, struct irqaction *action)
> > > > > +{
> > > > > + /*
> > > > > + * During suspend we must not call potentially unsafe irq handlers.
> > > > > + * See suspend_suspendable_actions.
> > > > > + */
> > > > > + if (unlikely(action->flags & IRQF_NO_ACTION))
> > > > > + return IRQ_NONE;
> > > >
> > > > Thomas was trying to avoid any new conditional code in the interrupt
> > > > handling path, that's why I added a suspended_action list in my
> > > > proposal.
> > > > Even if your 'unlikely' statement make things better I'm pretty sure it
> > > > adds some latency.
> > >
> > > I can see that we don't want to add more code here to keep things
> > > clean/pure, but I find it hard to believe that a single bit test and
> > > branch (for data that should be hot in the cache) are going to add
> > > measurable latency to a path that does pointer chasing to get to the
> > > irqaction in the first place. I could be wrong though, and I'm happy to
> > > benchmark.
> >
> > Again, I don't have enough experience to say this is (or isn't)
> > impacting irq handling latency, I'm just reporting what Thomas told me.
> >
> > >
> > > It would be possible to go for your list shuffling approach here while
> > > still keeping the flag internal and all the logic hidden away in
> > > kernel/irq/pm.c. I wasn't sure how actions could be manipulated during
> > > suspend, which made me wary of moving them to a separate list.
> >
> > Moving them to a temporary list on suspend and restoring them on
> > resume should not be a problem.
> > The only drawback I see is that actions might be reordered after the
> > first resume (anyway, relying on shared irq action order is dangerous
> > IMHO).
>
> We considered doing that too and saw some drawbacks (in addition to the
> reordering of actions you've mentioned). It added just too much complexity
> to the IRQ suspend-resume code.
>
> I, personally, would be fine with adding an IRQ flag to silence the
> warning mentioned by Alexandre. Something like IRQD_TIMER_SHARED that would
> be set automatically if someone requested IRQF_TIMER | IRQF_SHARED.
>
> Thoughts?
Even if the timer driver does that, we still require the other handlers
sharing the line to do the right thing across suspend, no? So either
their actions need to be masked at suspend time, or the handlers need to
detect when they're called during suspend and return early.
So for the flag at request time approach to work, all the drivers using
the interrupt would have to flag they're safe in that context.
I'm not averse to having that (only a few drivers shuold be affected and
we can sanity check them now).
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
