Re: [RFC][PATCH 2/2] PM: Rework handling of interrupts duringsuspend-resume

[Ingo Molnar]

* Rafael J. Wysocki <rjw@xxxxxxx> wrote:

> > > +void suspend_device_irqs(void)
> > > +{
> > > +	struct irq_desc *desc;
> > > +	int irq;
> > > +
> > > +	for_each_irq_desc(irq, desc) {
> > > +		unsigned long flags;
> > > +
> > > +		spin_lock_irqsave(&desc->lock, flags);
> > > +
> > > +		if (!desc->depth && desc->action
> > > +		    && !(desc->action->flags & IRQF_TIMER)) {
> > > +			desc->depth++;
> > > +			desc->status |= IRQ_DISABLED | IRQ_SUSPENDED;
> > > +			desc->chip->disable(irq);
> > > +		}
> > > +
> > > +		spin_unlock_irqrestore(&desc->lock, flags);
> > > +	}
> > > +
> > > +	for_each_irq_desc(irq, desc) {
> > > +		if (desc->status & IRQ_SUSPENDED)
> > > +			synchronize_irq(irq);
> > > +	}
> > 
> > Optimization/code-flow nit: a possibility might be to do a 
> > single loop, i.e. i think it's safe to couple the 
> > disable+sync bits [as in 99.99% of the cases there will be 
> > no in-execution irq handlers when we execute this.]
> 
> Well, Linus suggested to do it in a separate loop.  I'm fine 
> with both ways.

Linus, do you have a strong opinion about which variant we 
should use?

The two approaches are not completely equivalent, the variant 
suggested by Linus is a bit more 'atomic' - in that it first 
turns off everything, then looks for everything that needs to be 
synchronized.

OTOH, it _shouldnt_ make much of a difference on a correctly 
working system - we ought to be able to disable the irqs one by 
one and wait on any pending ones on the spot. Maybe if there was 
some implicit dependency between irq sources it would be more 
robust to do Linus's version.

Dunno ... no strong feelings either way.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/