Re: [PATCH V2 06/10] cpufreq: governor: Keep single copy of information common to policy->cpus

[Viresh Kumar <viresh.kumar@linaro.org>]

On 27-11-15, 12:56, Lucas Stach wrote:
> Sorry for being late to the party, as this has long been applied, but I
> stumbled upon this while looking at other stuff and wanted to express my
> finding.

No issues, we can still fix things if they have gone wrong.

> From a short look this change is totally bogus.

That's a bit shocking, after two people reviewed and tested it :)

> The *_dbs_timer functions are called from a workqueue on each CPU.

Right.

> Moving the time_stamp and the timer_mutex to a shared structure now
> causes a thundering herd issue on multicore systems.

You faced an issue or you just think that something is wrong ?

> All workqueues wake
> up at the same time with all but one of the worker bouncing on the
> contended mutex.

This is exactly what used to be done earlier, but it was designed
badly. For example, for ondemand governor, each CPU had a
struct od_cpu_dbs_info_s and we only used the od_cpu_dbs_info_s for
the 'policy->cpu'. So, even if all CPUs had this structure (i.e. the
time_stamp and the lock), we only used structure for policy->cpu.

> Also sharing the time_stamp causes all but the first worker to acquire
> the mutex to not properly update their sampling information, I guess.

Yeah, that's what we want per-policy.

> I didn't look to deeply into this.

Perhaps, yes.

-- 
viresh