Message-ID: <555DED03.9070002@linaro.org>
Date:	Thu, 21 May 2015 16:34:43 +0200
From:	Daniel Lezcano <daniel.lezcano@...aro.org>
To:	Ruchi Kandoi <kandoiruchi@...gle.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Vladimir Davydov <vdavydov@...allels.com>,
	Heinrich Schuchardt <xypron.glpk@....de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Kees Cook <keescook@...omium.org>,
	Konstantin Khlebnikov <khlebnikov@...dex-team.ru>,
	Davidlohr Bueso <dave@...olabs.net>, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 0/2] Adds cpu power accounting per-pid basis.

Hi Ruchi,

On 05/15/2015 02:12 AM, Ruchi Kandoi wrote:
> These patches add a mechanism which will accurately caculate the CPU power
> used by all the processes in the system. In order to account for the power
> used by all the processes a data field "cpu_power" has been added in the
> task_struct.

The term 'energy' makes more sense than 'power'.

> This field adds power for both the system as well as user
> time. cpu_power contains the total amount of charge(in uAmsec units) used

Why not use the Joules unit ?

> by the process. This model takes into account the frequency at which the
> process was running(i.e higher power for processes running at higher
> frequencies). It requires the cpufreq_stats module to be initialized with
> the current numbers for each of the CPU core at each frequency. This will
> be initialized during init time.

The energy task accounting is an interesting feature in my opinion. But 
your patchset does not deal with the power management hardware complexity.

If we reduce the scope of the task energy accounting to the cpu, we are 
facing several issues:

  * A cpu may be supposed to run at a specific OPP but it could share a 
clock line with another cpu which is in a higher frequency. So the 
frequency is actually at a higher rate than what is assumed

  * The firmware may override the cpufreq decisions

  * A process may be idle but its behavior forces the cpuidle governor 
to choose shallow states (that won't occur without the process). For 
example, the process is using very short timers, does a small processing 
and then go to sleep again waiting for the next timer expiration. The 
result will be a process having a low energy consumption but actually 
because of these timers, it will prevent the cpu to enter deep idle state

Beside that, the process may be soliciting a subsystem (another process 
or hardware) which consumes a lot of energy. That won't be accounted 
even if the process is responsible of this extra consumption.

And the last point is: how do you expect to have the energy numbers as 
nobody is willing to share them for their platform ?

> Ruchi Kandoi (2):
>    cpufreq_stats: Adds sysfs file
>      /sys/devices/system/cpu/cpufreq/current_in_state
>    sched: cpufreq: Adds a field cpu_power in the task_struct
>
>   drivers/cpufreq/cpufreq_stats.c | 191 +++++++++++++++++++++++++++++++++++++++-
>   include/linux/cpufreq.h         |   8 ++
>   include/linux/sched.h           |   2 +
>   kernel/fork.c                   |   1 +
>   kernel/sched/cputime.c          |   7 ++
>   5 files changed, 207 insertions(+), 2 deletions(-)



-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
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