lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 18 Sep 2014 14:32:25 -0400 (EDT) From: Nicolas Pitre <nicolas.pitre@...aro.org> To: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com> cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>, Daniel Lezcano <daniel.lezcano@...aro.org>, "Rafael J. Wysocki" <rjw@...ysocki.net>, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, linaro-kernel@...ts.linaro.org Subject: Re: [PATCH v2 1/2] sched: let the scheduler see CPU idle states On Thu, 18 Sep 2014, Paul E. McKenney wrote: > On Thu, Sep 04, 2014 at 11:32:09AM -0400, Nicolas Pitre wrote: > > From: Daniel Lezcano <daniel.lezcano@...aro.org> > > > > When the cpu enters idle, it stores the cpuidle state pointer in its > > struct rq instance which in turn could be used to make a better decision > > when balancing tasks. > > > > As soon as the cpu exits its idle state, the struct rq reference is > > cleared. > > > > There are a couple of situations where the idle state pointer could be changed > > while it is being consulted: > > > > 1. For x86/acpi with dynamic c-states, when a laptop switches from battery > > to AC that could result on removing the deeper idle state. The acpi driver > > triggers: > > 'acpi_processor_cst_has_changed' > > 'cpuidle_pause_and_lock' > > 'cpuidle_uninstall_idle_handler' > > 'kick_all_cpus_sync'. > > > > All cpus will exit their idle state and the pointed object will be set to > > NULL. > > > > 2. The cpuidle driver is unloaded. Logically that could happen but not > > in practice because the drivers are always compiled in and 95% of them are > > not coded to unregister themselves. In any case, the unloading code must > > call 'cpuidle_unregister_device', that calls 'cpuidle_pause_and_lock' > > leading to 'kick_all_cpus_sync' as mentioned above. > > > > A race can happen if we use the pointer and then one of these two scenarios > > occurs at the same moment. > > > > In order to be safe, the idle state pointer stored in the rq must be > > used inside a rcu_read_lock section where we are protected with the > > 'rcu_barrier' in the 'cpuidle_uninstall_idle_handler' function. The > > idle_get_state() and idle_put_state() accessors should be used to that > > effect. > > > > Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org> > > Signed-off-by: Nicolas Pitre <nico@...aro.org> > > --- > > drivers/cpuidle/cpuidle.c | 6 ++++++ > > kernel/sched/idle.c | 6 ++++++ > > kernel/sched/sched.h | 39 +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 51 insertions(+) > > > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c > > index ee9df5e3f5..530e3055a2 100644 > > --- a/drivers/cpuidle/cpuidle.c > > +++ b/drivers/cpuidle/cpuidle.c > > @@ -225,6 +225,12 @@ void cpuidle_uninstall_idle_handler(void) > > initialized = 0; > > kick_all_cpus_sync(); > > } > > + > > + /* > > + * Make sure external observers (such as the scheduler) > > + * are done looking at pointed idle states. > > + */ > > + rcu_barrier(); > > Actually, all rcu_barrier() does is to make sure that all previously > queued RCU callbacks have been invoked. And given the current > implementation, if there are no callbacks queued anywhere in the system, > rcu_barrier() is an extended no-op. "Has CPU 0 any callbacks?" "Nope!" > "Has CPU 1 any callbacks?" "Nope!" ... "Has CPU nr_cpu_ids-1 any > callbacks?" "Nope!" "OK, done!" > > This is all done with the current task looking at per-CPU data structures, > with no interaction with the scheduler and with no need to actually make > those other CPUs do anything. > > So what is it that you really need to do here? In short, we don't want the cpufreq data to go away (see the 2 scenarios above) while the scheduler is looking at it. The scheduler uses the provided accessors (see patch 2/2) so we can put any protection mechanism we want in them. A simple spinlock could do just as well which should be good enough. Nicolas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists