On Mon, 18 Jan 2016, Daniel Lezcano wrote:

On 01/08/2016 04:43 PM, Thomas Gleixner wrote:

quoted

The above is completely unprotected against interrupts being setup or even
freed concurrently.

Aside of that, you call that setup function in setup_irq for each action()
and
here you call it only for the first one.

I went through the different comments and almost finished the changes but I
think the 'register_ops' approach, which happens after some irq were setup,
introduces some useless complexity and because of the desc lock section, the
ops can't do memory allocation.

You can't protect that with desc_lock. You need to take the sparse_irq_lock,
which is a mutex, to protect the irq desc walk.

Before going further, I am wondering if declaring the irq_timings_ops
statically (read without 'register_ops' - hence without a init time
dependency) and calling the init/free ops from alloc_desc/free_desc wouldn't
be cleaner and simpler.

Then you don't need those ops at all. You can make it simple function calls,
which get compiled out if that stuff is not enabled.

Thanks,

	tglx