[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.11.1601201634520.3575@nanos>
Date: Wed, 20 Jan 2016 16:41:10 +0100 (CET)
From: Thomas Gleixner <tglx@...utronix.de>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
cc: peterz@...radead.org, rafael@...nel.org, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, nicolas.pitre@...aro.org,
vincent.guittot@...aro.org
Subject: Re: [RFC PATCH 2/2] sched: idle: IRQ based next prediction for idle
period
On Mon, 18 Jan 2016, Daniel Lezcano wrote:
> On 01/08/2016 04:43 PM, Thomas Gleixner wrote:
> > 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
Powered by blists - more mailing lists