Message ID | 1743951.Hhdov0yvNG@vostro.rjw.lan (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
On Sat, Feb 27, 2016 at 01:08:02AM +0100, Rafael J. Wysocki wrote: > @@ -95,18 +98,20 @@ EXPORT_SYMBOL_GPL(cpufreq_set_update_uti > * > * This function is called by the scheduler on every invocation of > * update_load_avg() on the CPU whose utilization is being updated. > + * > + * It can only be called from RCU-sched read-side critical sections. > */ > void cpufreq_update_util(u64 time, unsigned long util, unsigned long max) > { > struct update_util_data *data; > > - rcu_read_lock(); > - Maybe, just because I'm paranoid, add something like: #ifdef CONFIG_LOCKDEP WARN_ON(debug_locks && !rcu_read_lock_sched_held()); #endif > data = rcu_dereference(*this_cpu_ptr(&cpufreq_update_util_data)); > - if (data && data->func) > + /* > + * If this isn't inside of an RCU-sched read-side critical section, data > + * may become NULL after the check below. > + */ > + if (data) > data->func(data, time, util, max); > - > - rcu_read_unlock(); > } -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 1, 2016 at 1:57 PM, Peter Zijlstra <peterz@infradead.org> wrote: > On Sat, Feb 27, 2016 at 01:08:02AM +0100, Rafael J. Wysocki wrote: >> @@ -95,18 +98,20 @@ EXPORT_SYMBOL_GPL(cpufreq_set_update_uti >> * >> * This function is called by the scheduler on every invocation of >> * update_load_avg() on the CPU whose utilization is being updated. >> + * >> + * It can only be called from RCU-sched read-side critical sections. >> */ >> void cpufreq_update_util(u64 time, unsigned long util, unsigned long max) >> { >> struct update_util_data *data; >> >> - rcu_read_lock(); >> - > > Maybe, just because I'm paranoid, add something like: > > #ifdef CONFIG_LOCKDEP > WARN_ON(debug_locks && !rcu_read_lock_sched_held()); > #endif OK -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux-pm/drivers/cpufreq/cpufreq.c =================================================================== --- linux-pm.orig/drivers/cpufreq/cpufreq.c +++ linux-pm/drivers/cpufreq/cpufreq.c @@ -77,12 +77,15 @@ static DEFINE_PER_CPU(struct update_util * to call from cpufreq_update_util(). That function will be called from an RCU * read-side critical section, so it must not sleep. * - * Callers must use RCU callbacks to free any memory that might be accessed - * via the old update_util_data pointer or invoke synchronize_rcu() right after - * this function to avoid use-after-free. + * Callers must use RCU-sched callbacks to free any memory that might be + * accessed via the old update_util_data pointer or invoke synchronize_sched() + * right after this function to avoid use-after-free. */ void cpufreq_set_update_util_data(int cpu, struct update_util_data *data) { + if (WARN_ON(data && !data->func)) + return; + rcu_assign_pointer(per_cpu(cpufreq_update_util_data, cpu), data); } EXPORT_SYMBOL_GPL(cpufreq_set_update_util_data); @@ -95,18 +98,20 @@ EXPORT_SYMBOL_GPL(cpufreq_set_update_uti * * This function is called by the scheduler on every invocation of * update_load_avg() on the CPU whose utilization is being updated. + * + * It can only be called from RCU-sched read-side critical sections. */ void cpufreq_update_util(u64 time, unsigned long util, unsigned long max) { struct update_util_data *data; - rcu_read_lock(); - data = rcu_dereference(*this_cpu_ptr(&cpufreq_update_util_data)); - if (data && data->func) + /* + * If this isn't inside of an RCU-sched read-side critical section, data + * may become NULL after the check below. + */ + if (data) data->func(data, time, util, max); - - rcu_read_unlock(); } /* Flag to suspend/resume CPUFreq governors */ Index: linux-pm/drivers/cpufreq/cpufreq_governor.c =================================================================== --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.c +++ linux-pm/drivers/cpufreq/cpufreq_governor.c @@ -280,7 +280,7 @@ static inline void gov_clear_update_util for_each_cpu(i, policy->cpus) cpufreq_set_update_util_data(i, NULL); - synchronize_rcu(); + synchronize_sched(); } static void gov_cancel_work(struct cpufreq_policy *policy) Index: linux-pm/drivers/cpufreq/intel_pstate.c =================================================================== --- linux-pm.orig/drivers/cpufreq/intel_pstate.c +++ linux-pm/drivers/cpufreq/intel_pstate.c @@ -1171,7 +1171,7 @@ static void intel_pstate_stop_cpu(struct pr_debug("intel_pstate: CPU %d exiting\n", cpu_num); cpufreq_set_update_util_data(cpu_num, NULL); - synchronize_rcu(); + synchronize_sched(); if (hwp_active) return; @@ -1429,7 +1429,7 @@ out: for_each_online_cpu(cpu) { if (all_cpu_data[cpu]) { cpufreq_set_update_util_data(cpu, NULL); - synchronize_rcu(); + synchronize_sched(); kfree(all_cpu_data[cpu]); } }