Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

cpufreq: stats: Make the stats code non-modular

The modularity of cpufreq_stats is quite problematic.

First off, the usage of policy notifiers for the initialization
and cleanup in the cpufreq_stats module is inherently racy with
respect to CPU offline/online and the initialization and cleanup
of the cpufreq driver.

Second, fast frequency switching (used by the schedutil governor)
cannot be enabled if any transition notifiers are registered, so
if the cpufreq_stats module (that registers a transition notifier
for updating transition statistics) is loaded, the schedutil governor
cannot use fast frequency switching.

On the other hand, allowing cpufreq_stats to be built as a module
doesn't really add much value. Arguably, there's not much reason
for that code to be modular at all.

For the above reasons, make the cpufreq stats code non-modular,
modify the core to invoke functions provided by that code directly
and drop the notifiers from it.

Make the stats sysfs attributes appear empty if fast frequency
switching is enabled as the statistics will not be updated in that
case anyway (and returning -EBUSY from those attributes breaks
powertop).

While at it, clean up Kconfig help for the CPU_FREQ_STAT and
CPU_FREQ_STAT_DETAILS options.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

+41 -142
+4 -9
drivers/cpufreq/Kconfig
··· 31 31 depends on THERMAL 32 32 33 33 config CPU_FREQ_STAT 34 - tristate "CPU frequency translation statistics" 34 + bool "CPU frequency transition statistics" 35 35 default y 36 36 help 37 - This driver exports CPU frequency statistics information through sysfs 38 - file system. 39 - 40 - To compile this driver as a module, choose M here: the 41 - module will be called cpufreq_stats. 37 + Export CPU frequency statistics information through sysfs. 42 38 43 39 If in doubt, say N. 44 40 45 41 config CPU_FREQ_STAT_DETAILS 46 - bool "CPU frequency translation statistics details" 42 + bool "CPU frequency transition statistics details" 47 43 depends on CPU_FREQ_STAT 48 44 help 49 - This will show detail CPU frequency translation table in sysfs file 50 - system. 45 + Show detailed CPU frequency transition table in sysfs. 51 46 52 47 If in doubt, say N. 53 48
+4
drivers/cpufreq/cpufreq.c
··· 347 347 pr_debug("FREQ: %lu - CPU: %lu\n", 348 348 (unsigned long)freqs->new, (unsigned long)freqs->cpu); 349 349 trace_cpu_frequency(freqs->new, freqs->cpu); 350 + cpufreq_stats_record_transition(policy, freqs->new); 350 351 srcu_notifier_call_chain(&cpufreq_transition_notifier_list, 351 352 CPUFREQ_POSTCHANGE, freqs); 352 353 if (likely(policy) && likely(policy->cpu == freqs->cpu)) ··· 1109 1108 CPUFREQ_REMOVE_POLICY, policy); 1110 1109 1111 1110 down_write(&policy->rwsem); 1111 + cpufreq_stats_free_table(policy); 1112 1112 cpufreq_remove_dev_symlink(policy); 1113 1113 kobj = &policy->kobj; 1114 1114 cmp = &policy->kobj_unregister; ··· 1264 1262 ret = cpufreq_add_dev_interface(policy); 1265 1263 if (ret) 1266 1264 goto out_exit_policy; 1265 + 1266 + cpufreq_stats_create_table(policy); 1267 1267 blocking_notifier_call_chain(&cpufreq_policy_notifier_list, 1268 1268 CPUFREQ_CREATE_POLICY, policy); 1269 1269
+21 -133
drivers/cpufreq/cpufreq_stats.c
··· 15 15 #include <linux/slab.h> 16 16 #include <linux/cputime.h> 17 17 18 - static spinlock_t cpufreq_stats_lock; 18 + static DEFINE_SPINLOCK(cpufreq_stats_lock); 19 19 20 20 struct cpufreq_stats { 21 21 unsigned int total_trans; ··· 52 52 ssize_t len = 0; 53 53 int i; 54 54 55 + if (policy->fast_switch_enabled) 56 + return 0; 57 + 55 58 cpufreq_stats_update(stats); 56 59 for (i = 0; i < stats->state_num; i++) { 57 60 len += sprintf(buf + len, "%u %llu\n", stats->freq_table[i], ··· 70 67 struct cpufreq_stats *stats = policy->stats; 71 68 ssize_t len = 0; 72 69 int i, j; 70 + 71 + if (policy->fast_switch_enabled) 72 + return 0; 73 73 74 74 len += snprintf(buf + len, PAGE_SIZE - len, " From : To\n"); 75 75 len += snprintf(buf + len, PAGE_SIZE - len, " : "); ··· 136 130 return -1; 137 131 } 138 132 139 - static void __cpufreq_stats_free_table(struct cpufreq_policy *policy) 133 + void cpufreq_stats_free_table(struct cpufreq_policy *policy) 140 134 { 141 135 struct cpufreq_stats *stats = policy->stats; 142 136 ··· 152 146 policy->stats = NULL; 153 147 } 154 148 155 - static void cpufreq_stats_free_table(unsigned int cpu) 156 - { 157 - struct cpufreq_policy *policy; 158 - 159 - policy = cpufreq_cpu_get(cpu); 160 - if (!policy) 161 - return; 162 - 163 - __cpufreq_stats_free_table(policy); 164 - 165 - cpufreq_cpu_put(policy); 166 - } 167 - 168 - static int __cpufreq_stats_create_table(struct cpufreq_policy *policy) 149 + void cpufreq_stats_create_table(struct cpufreq_policy *policy) 169 150 { 170 151 unsigned int i = 0, count = 0, ret = -ENOMEM; 171 152 struct cpufreq_stats *stats; ··· 163 170 /* We need cpufreq table for creating stats table */ 164 171 table = cpufreq_frequency_get_table(cpu); 165 172 if (unlikely(!table)) 166 - return 0; 173 + return; 167 174 168 175 /* stats already initialized */ 169 176 if (policy->stats) 170 - return -EEXIST; 177 + return; 171 178 172 179 stats = kzalloc(sizeof(*stats), GFP_KERNEL); 173 180 if (!stats) 174 - return -ENOMEM; 181 + return; 175 182 176 183 /* Find total allocation size */ 177 184 cpufreq_for_each_valid_entry(pos, table) ··· 208 215 policy->stats = stats; 209 216 ret = sysfs_create_group(&policy->kobj, &stats_attr_group); 210 217 if (!ret) 211 - return 0; 218 + return; 212 219 213 220 /* We failed, release resources */ 214 221 policy->stats = NULL; 215 222 kfree(stats->time_in_state); 216 223 free_stat: 217 224 kfree(stats); 218 - 219 - return ret; 220 225 } 221 226 222 - static void cpufreq_stats_create_table(unsigned int cpu) 227 + void cpufreq_stats_record_transition(struct cpufreq_policy *policy, 228 + unsigned int new_freq) 223 229 { 224 - struct cpufreq_policy *policy; 225 - 226 - /* 227 - * "likely(!policy)" because normally cpufreq_stats will be registered 228 - * before cpufreq driver 229 - */ 230 - policy = cpufreq_cpu_get(cpu); 231 - if (likely(!policy)) 232 - return; 233 - 234 - __cpufreq_stats_create_table(policy); 235 - 236 - cpufreq_cpu_put(policy); 237 - } 238 - 239 - static int cpufreq_stat_notifier_policy(struct notifier_block *nb, 240 - unsigned long val, void *data) 241 - { 242 - int ret = 0; 243 - struct cpufreq_policy *policy = data; 244 - 245 - if (val == CPUFREQ_CREATE_POLICY) 246 - ret = __cpufreq_stats_create_table(policy); 247 - else if (val == CPUFREQ_REMOVE_POLICY) 248 - __cpufreq_stats_free_table(policy); 249 - 250 - return ret; 251 - } 252 - 253 - static int cpufreq_stat_notifier_trans(struct notifier_block *nb, 254 - unsigned long val, void *data) 255 - { 256 - struct cpufreq_freqs *freq = data; 257 - struct cpufreq_policy *policy = cpufreq_cpu_get(freq->cpu); 258 - struct cpufreq_stats *stats; 230 + struct cpufreq_stats *stats = policy->stats; 259 231 int old_index, new_index; 260 232 261 - if (!policy) { 262 - pr_err("%s: No policy found\n", __func__); 263 - return 0; 264 - } 265 - 266 - if (val != CPUFREQ_POSTCHANGE) 267 - goto put_policy; 268 - 269 - if (!policy->stats) { 233 + if (!stats) { 270 234 pr_debug("%s: No stats found\n", __func__); 271 - goto put_policy; 235 + return; 272 236 } 273 - 274 - stats = policy->stats; 275 237 276 238 old_index = stats->last_index; 277 - new_index = freq_table_get_index(stats, freq->new); 239 + new_index = freq_table_get_index(stats, new_freq); 278 240 279 241 /* We can't do stats->time_in_state[-1]= .. */ 280 - if (old_index == -1 || new_index == -1) 281 - goto put_policy; 282 - 283 - if (old_index == new_index) 284 - goto put_policy; 242 + if (old_index == -1 || new_index == -1 || old_index == new_index) 243 + return; 285 244 286 245 cpufreq_stats_update(stats); 287 246 ··· 242 297 stats->trans_table[old_index * stats->max_state + new_index]++; 243 298 #endif 244 299 stats->total_trans++; 245 - 246 - put_policy: 247 - cpufreq_cpu_put(policy); 248 - return 0; 249 300 } 250 - 251 - static struct notifier_block notifier_policy_block = { 252 - .notifier_call = cpufreq_stat_notifier_policy 253 - }; 254 - 255 - static struct notifier_block notifier_trans_block = { 256 - .notifier_call = cpufreq_stat_notifier_trans 257 - }; 258 - 259 - static int __init cpufreq_stats_init(void) 260 - { 261 - int ret; 262 - unsigned int cpu; 263 - 264 - spin_lock_init(&cpufreq_stats_lock); 265 - ret = cpufreq_register_notifier(&notifier_policy_block, 266 - CPUFREQ_POLICY_NOTIFIER); 267 - if (ret) 268 - return ret; 269 - 270 - for_each_online_cpu(cpu) 271 - cpufreq_stats_create_table(cpu); 272 - 273 - ret = cpufreq_register_notifier(&notifier_trans_block, 274 - CPUFREQ_TRANSITION_NOTIFIER); 275 - if (ret) { 276 - cpufreq_unregister_notifier(&notifier_policy_block, 277 - CPUFREQ_POLICY_NOTIFIER); 278 - for_each_online_cpu(cpu) 279 - cpufreq_stats_free_table(cpu); 280 - return ret; 281 - } 282 - 283 - return 0; 284 - } 285 - static void __exit cpufreq_stats_exit(void) 286 - { 287 - unsigned int cpu; 288 - 289 - cpufreq_unregister_notifier(&notifier_policy_block, 290 - CPUFREQ_POLICY_NOTIFIER); 291 - cpufreq_unregister_notifier(&notifier_trans_block, 292 - CPUFREQ_TRANSITION_NOTIFIER); 293 - for_each_online_cpu(cpu) 294 - cpufreq_stats_free_table(cpu); 295 - } 296 - 297 - MODULE_AUTHOR("Zou Nan hai <nanhai.zou@intel.com>"); 298 - MODULE_DESCRIPTION("Export cpufreq stats via sysfs"); 299 - MODULE_LICENSE("GPL"); 300 - 301 - module_init(cpufreq_stats_init); 302 - module_exit(cpufreq_stats_exit);
+12
include/linux/cpufreq.h
··· 185 185 static inline void disable_cpufreq(void) { } 186 186 #endif 187 187 188 + #ifdef CONFIG_CPU_FREQ_STAT 189 + void cpufreq_stats_create_table(struct cpufreq_policy *policy); 190 + void cpufreq_stats_free_table(struct cpufreq_policy *policy); 191 + void cpufreq_stats_record_transition(struct cpufreq_policy *policy, 192 + unsigned int new_freq); 193 + #else 194 + static inline void cpufreq_stats_create_table(struct cpufreq_policy *policy) { } 195 + static inline void cpufreq_stats_free_table(struct cpufreq_policy *policy) { } 196 + static inline void cpufreq_stats_record_transition(struct cpufreq_policy *policy, 197 + unsigned int new_freq) { } 198 + #endif /* CONFIG_CPU_FREQ_STAT */ 199 + 188 200 /********************************************************************* 189 201 * CPUFREQ DRIVER INTERFACE * 190 202 *********************************************************************/