On Tue, Jan 15, 2019 at 10:15:01AM +0000, Patrick Bellasi wrote: > @@ -835,6 +954,28 @@ static void uclamp_bucket_inc(struct uclamp_se *uc_se, unsigned int clamp_id, > } while (!atomic_long_try_cmpxchg(&uc_maps[bucket_id].adata, > &uc_map_old.data, uc_map_new.data)); > > + /* > + * Ensure each CPU tracks the correct value for this clamp bucket. > + * This initialization of per-CPU variables is required only when a > + * clamp value is requested for the first time from a slow-path. > + */ I'm confused; why is this needed? > + if (unlikely(!uc_map_old.se_count)) { > + for_each_possible_cpu(cpu) { > + struct uclamp_cpu *uc_cpu = > + &cpu_rq(cpu)->uclamp[clamp_id]; > + > + /* CPU's tasks count must be 0 for free buckets */ > + SCHED_WARN_ON(uc_cpu->bucket[bucket_id].tasks); > + if (unlikely(uc_cpu->bucket[bucket_id].tasks)) > + uc_cpu->bucket[bucket_id].tasks = 0; > + > + /* Minimize cache lines invalidation */ > + if (uc_cpu->bucket[bucket_id].value == bucket_value) > + continue; > + uc_cpu->bucket[bucket_id].value = bucket_value; > + } > + } > + > uc_se->value = clamp_value; > uc_se->bucket_id = bucket_id; >