Re: Linux 4.9.6 ( Restore IO-APIC irq_chip retrigger callback , breaks my box )

[Thomas Gleixner]

On Mon, 13 Feb 2017, Mike Galbraith wrote:
>  kernel/time/tick-broadcast.c |    5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> --- a/kernel/time/tick-broadcast.c
> +++ b/kernel/time/tick-broadcast.c
> @@ -357,6 +357,7 @@ void tick_broadcast_control(enum tick_br
>  	struct clock_event_device *bc, *dev;
>  	struct tick_device *td;
>  	int cpu, bc_stopped;
> +	unsigned long flags;
>  
>  	td = this_cpu_ptr(&tick_cpu_device);
>  	dev = td->evtdev;
> @@ -370,7 +371,7 @@ void tick_broadcast_control(enum tick_br
>  	if (!tick_device_is_functional(dev))
>  		return;
>  
> -	raw_spin_lock(&tick_broadcast_lock);
> +	raw_spin_lock_irqsave(&tick_broadcast_lock, flags);
>  	cpu = smp_processor_id();
>  	bc = tick_broadcast_device.evtdev;
>  	bc_stopped = cpumask_empty(tick_broadcast_mask);
> @@ -420,7 +421,7 @@ void tick_broadcast_control(enum tick_br
>  				tick_broadcast_setup_oneshot(bc);
>  		}
>  	}
> -	raw_spin_unlock(&tick_broadcast_lock);
> +	raw_spin_unlock_irqrestore(&tick_broadcast_lock, flags);

That cures the lockdep splat, but the comment above
tick_broadcast_control() says:

* Called with interrupts disabled, so clockevents_lock is not
* required here because the local clock event device cannot go away
* under us.

So if we want to relax the calling convention, then we need to take the
lock early.  Otherwise it's unsafe to fiddle with the local clock event
device.

The calling convention was broken with the following commit:

    29d7bbada98e intel_idle: Remove superfluous SMP fuction call

So we could fix it at the call site, but making the core more robust is the
better solution.

I'll fix it up.

Thanks,

	tglx