CPU hotplug and chained interrupts on x86

CPU hotplug and chained interrupts on x86

[Mika Westerberg]

Hi Thomas,

On Intel Braswell system (this affect others if they are using chained
interrupts on x86) both CPUs can handle interrupts which trigger when
GPIO line changes state if programmed to do so.

We have SD-card card detection signal that is connected to a GPIO line:

  # cat /proc/interrupts 
              CPU0       CPU1       
   ...
   304:          0          0  chv-gpio   50  80860F14:01 cd

This works fine until the other CPU is offlined.

  # echo 0 > /sys/devices/system/cpu/cpu1/online

I modified arch/x86/kernel/irq.c:fixup_irqs() slightly so that it calls
print_IO_APICs() at the end to be able to see how interrupts are routed
after the CPU is offlined (below lists entries related to the four
interrupts used by the GPIO controller):

  IOAPIC 0:
   pin30, enabled , level, low , V(52), IRR(0), S(0), logical , D(03), M(1)
   pin31, enabled , level, low , V(42), IRR(0), S(0), logical , D(03), M(1)
   pin32, enabled , level, low , V(62), IRR(0), S(0), logical , D(03), M(1)
   pin5b, enabled , level, low , V(72), IRR(0), S(0), logical , D(03), M(1)

The destination mask D(03) says that the interrupt can be delivered to any
of the two CPUs and mode M(1) says to deliver it to lowest priority CPU
among the list.

Now if I plug/unplug the card I may get few interrupts to CPU0 but rest
of the interrupts never happen. Probably because IO-APIC forwards them
to the lowest priority CPU which is offline at this point.

There is following check in fixup_irqs():

	if (!irq_has_action(irq) || irqd_is_per_cpu(data) ||
	    cpumask_subset(affinity, cpu_online_mask)) {
		raw_spin_unlock(&desc->lock);
		continue;
	}   

If an interrupt is requested by a driver it will force new affinity and
everything works fine. However if the interrupt is chained (it does not
have ->action) this is skipped and the current affinity remains.

I'm able to work this around by forcing the affinity here for the 4 chained
interrupts. However, I'm not quite sure what would the proper fix be.

We could detect here if the interrupt is chained but there seems to be
no easy way to determine it currently so we would need to add a new flag
to desc->status_use_accessors that gets set in __irq_do_set_handler()
when is_chained is 1.

Alternative I could implement ->irq_set_affinity() in the GPIO driver in
question [1] which always calls directly parent chip's ->irq_set_affinity()
but I'm not sure if that is allowed.

Any ideas how to get this properly fixed?

Thanks in advance.

[1] drivers/pinctrl/intel/pinctrl-cherryview.c

Re: CPU hotplug and chained interrupts on x86

[Thomas Gleixner]

On Thu, 1 Oct 2015, Mika Westerberg wrote:
> Now if I plug/unplug the card I may get few interrupts to CPU0 but rest
> of the interrupts never happen. Probably because IO-APIC forwards them
> to the lowest priority CPU which is offline at this point.
> 
> There is following check in fixup_irqs():
> 
> 	if (!irq_has_action(irq) || irqd_is_per_cpu(data) ||
> 	    cpumask_subset(affinity, cpu_online_mask)) {
> 		raw_spin_unlock(&desc->lock);
> 		continue;
> 	}   
> 
> If an interrupt is requested by a driver it will force new affinity and
> everything works fine. However if the interrupt is chained (it does not
> have ->action) this is skipped and the current affinity remains.
> 
> We could detect here if the interrupt is chained but there seems to be
> no easy way to determine it currently so we would need to add a new flag
> to desc->status_use_accessors that gets set in __irq_do_set_handler()
> when is_chained is 1.

Either there or in irq_data. Need to look at it in detail.
 
> Alternative I could implement ->irq_set_affinity() in the GPIO driver in
> question [1] which always calls directly parent chip's ->irq_set_affinity()
> but I'm not sure if that is allowed.

I rather prefer to avoid that.

Thanks,

	tglx

Re: CPU hotplug and chained interrupts on x86

[Jiang Liu]

On 2015/10/1 22:31, Thomas Gleixner wrote:
> On Thu, 1 Oct 2015, Mika Westerberg wrote:
>> Now if I plug/unplug the card I may get few interrupts to CPU0 but rest
>> of the interrupts never happen. Probably because IO-APIC forwards them
>> to the lowest priority CPU which is offline at this point.
>>
>> There is following check in fixup_irqs():
>>
>> 	if (!irq_has_action(irq) || irqd_is_per_cpu(data) ||
>> 	    cpumask_subset(affinity, cpu_online_mask)) {
>> 		raw_spin_unlock(&desc->lock);
>> 		continue;
>> 	}   
>>
>> If an interrupt is requested by a driver it will force new affinity and
>> everything works fine. However if the interrupt is chained (it does not
>> have ->action) this is skipped and the current affinity remains.
>>
>> We could detect here if the interrupt is chained but there seems to be
>> no easy way to determine it currently so we would need to add a new flag
>> to desc->status_use_accessors that gets set in __irq_do_set_handler()
>> when is_chained is 1.
> 
> Either there or in irq_data. Need to look at it in detail.
Currently  we have no flag for chained, I suggested to add one dedicated
flag for it.

>  
>> Alternative I could implement ->irq_set_affinity() in the GPIO driver in
>> question [1] which always calls directly parent chip's ->irq_set_affinity()
>> but I'm not sure if that is allowed.
> 
> I rather prefer to avoid that.
We should report chained state and parent irq, so user and irqbalance
may make smarter decision based on those info.
Thanks!
Gerry

> 
> Thanks,
> 
> 	tglx
> 

Re: CPU hotplug and chained interrupts on x86

[Thomas Gleixner]

On Thu, 1 Oct 2015, Thomas Gleixner wrote:
> On Thu, 1 Oct 2015, Mika Westerberg wrote:
> > Now if I plug/unplug the card I may get few interrupts to CPU0 but rest
> > of the interrupts never happen. Probably because IO-APIC forwards them
> > to the lowest priority CPU which is offline at this point.
> > 
> > There is following check in fixup_irqs():
> > 
> > 	if (!irq_has_action(irq) || irqd_is_per_cpu(data) ||
> > 	    cpumask_subset(affinity, cpu_online_mask)) {
> > 		raw_spin_unlock(&desc->lock);
> > 		continue;
> > 	}   
> > 
> > If an interrupt is requested by a driver it will force new affinity and
> > everything works fine. However if the interrupt is chained (it does not
> > have ->action) this is skipped and the current affinity remains.
> > 
> > We could detect here if the interrupt is chained but there seems to be
> > no easy way to determine it currently so we would need to add a new flag
> > to desc->status_use_accessors that gets set in __irq_do_set_handler()
> > when is_chained is 1.
> 
> Either there or in irq_data. Need to look at it in detail.

desc->status_use_accessors is the place where this wants to go.

Thanks,

	tglx

Re: CPU hotplug and chained interrupts on x86

[Mika Westerberg]

On Thu, Oct 01, 2015 at 11:45:23PM +0200, Thomas Gleixner wrote:
> On Thu, 1 Oct 2015, Thomas Gleixner wrote:
> > On Thu, 1 Oct 2015, Mika Westerberg wrote:
> > > Now if I plug/unplug the card I may get few interrupts to CPU0 but rest
> > > of the interrupts never happen. Probably because IO-APIC forwards them
> > > to the lowest priority CPU which is offline at this point.
> > > 
> > > There is following check in fixup_irqs():
> > > 
> > > 	if (!irq_has_action(irq) || irqd_is_per_cpu(data) ||
> > > 	    cpumask_subset(affinity, cpu_online_mask)) {
> > > 		raw_spin_unlock(&desc->lock);
> > > 		continue;
> > > 	}   
> > > 
> > > If an interrupt is requested by a driver it will force new affinity and
> > > everything works fine. However if the interrupt is chained (it does not
> > > have ->action) this is skipped and the current affinity remains.
> > > 
> > > We could detect here if the interrupt is chained but there seems to be
> > > no easy way to determine it currently so we would need to add a new flag
> > > to desc->status_use_accessors that gets set in __irq_do_set_handler()
> > > when is_chained is 1.
> > 
> > Either there or in irq_data. Need to look at it in detail.
> 
> desc->status_use_accessors is the place where this wants to go.

Thank you.

I'll prepare a patch fixing this shortly.