All of lore.kernel.org
 help / color / mirror / Atom feed
From: Glauber Costa <glommer@parallels.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: <cgroups@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Tejun Heo <tj@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Turner <pjt@google.com>
Subject: Re: [PATCH v5 11/11] sched: introduce cgroup file stat_percpu
Date: Thu, 10 Jan 2013 01:10:02 +0400	[thread overview]
Message-ID: <50EDDCAA.6070004@parallels.com> (raw)
In-Reply-To: <20130109124220.ad9f1a54.akpm@linux-foundation.org>

On 01/10/2013 12:42 AM, Andrew Morton wrote:
> On Wed,  9 Jan 2013 15:45:38 +0400
> Glauber Costa <glommer@parallels.com> wrote:
> 
>> The file cpu.stat_percpu will show various scheduler related
>> information, that are usually available to the top level through other
>> files.
>>
>> For instance, most of the meaningful data in /proc/stat is presented
>> here. Given this file, a container can easily construct a local copy of
>> /proc/stat for internal consumption.
>>
>> The data we export is comprised of:
>> * all the tick information, previously available only through cpuacct,
>>   like user time, system time, etc.
>>
>> * wait time, which can be used to construct analogous information to
>>   steal time in hypervisors,
>>
>> * nr_switches and nr_running, which are cgroup-local versions of
>>   their global counterparts.
>>
>> The file includes a header, so fields can come and go if needed.
> 
> Please update this changelog to fully describe the proposed userspace
> interfaces.  That means full pathnames and example output. 
> Understanding these interfaces is the most important part of reviewing
> this patchset, so this info should be prominent.
> 
> Also, this patchset appears to alter (or remove?) existing userspace
> interfaces?  If so, please clearly describe those alterations and also
> share your thinking on the back-compatibility issues.
> 
> Also, I'm not seeing any changes to Docmentation/ in this patchset. 
> How do we explain the interface to our users?
> 
> 
> From a quick read, it appears that the output will be something along
> the lines of:
> 
> user nice system irq softirq guest guest_nice wait nr_switches nr_running
> cpu0 nn nn nn nn nn ...
> cpu1 nn nn nn nn nn ...
> 
> which looks pretty terrible.  Apart from being very hard to read, it
> means that we can never remove fields.  A nicer and more extensible
> interface would be
> 
> cpu:0 nice:nn system:nn irq:nn ...
> 

Ok.

The actual output format is what matters the least to me, so I can
change to whatever pleases you guys.

I just don't how is it that we can never remove fields. My very
motivation for adding a header in the first place, was to give us
ability to extend this.

For a next round, I will include a Documentation file as you requested.
I could, for instance, explicitly mention that people parsing this
should first query the first line, which constitutes a header.

The main advantage I see in this approach, is that there is way less
data to be written using a header. Although your way works, it means we
will write the strings "nice", "system", etc. #cpu times. Quite a waste.

> 


WARNING: multiple messages have this Message-ID (diff)
From: Glauber Costa <glommer@parallels.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
	Tejun Heo <tj@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Turner <pjt@google.com>
Subject: Re: [PATCH v5 11/11] sched: introduce cgroup file stat_percpu
Date: Thu, 10 Jan 2013 01:10:02 +0400	[thread overview]
Message-ID: <50EDDCAA.6070004@parallels.com> (raw)
In-Reply-To: <20130109124220.ad9f1a54.akpm@linux-foundation.org>

On 01/10/2013 12:42 AM, Andrew Morton wrote:
> On Wed,  9 Jan 2013 15:45:38 +0400
> Glauber Costa <glommer@parallels.com> wrote:
> 
>> The file cpu.stat_percpu will show various scheduler related
>> information, that are usually available to the top level through other
>> files.
>>
>> For instance, most of the meaningful data in /proc/stat is presented
>> here. Given this file, a container can easily construct a local copy of
>> /proc/stat for internal consumption.
>>
>> The data we export is comprised of:
>> * all the tick information, previously available only through cpuacct,
>>   like user time, system time, etc.
>>
>> * wait time, which can be used to construct analogous information to
>>   steal time in hypervisors,
>>
>> * nr_switches and nr_running, which are cgroup-local versions of
>>   their global counterparts.
>>
>> The file includes a header, so fields can come and go if needed.
> 
> Please update this changelog to fully describe the proposed userspace
> interfaces.  That means full pathnames and example output. 
> Understanding these interfaces is the most important part of reviewing
> this patchset, so this info should be prominent.
> 
> Also, this patchset appears to alter (or remove?) existing userspace
> interfaces?  If so, please clearly describe those alterations and also
> share your thinking on the back-compatibility issues.
> 
> Also, I'm not seeing any changes to Docmentation/ in this patchset. 
> How do we explain the interface to our users?
> 
> 
> From a quick read, it appears that the output will be something along
> the lines of:
> 
> user nice system irq softirq guest guest_nice wait nr_switches nr_running
> cpu0 nn nn nn nn nn ...
> cpu1 nn nn nn nn nn ...
> 
> which looks pretty terrible.  Apart from being very hard to read, it
> means that we can never remove fields.  A nicer and more extensible
> interface would be
> 
> cpu:0 nice:nn system:nn irq:nn ...
> 

Ok.

The actual output format is what matters the least to me, so I can
change to whatever pleases you guys.

I just don't how is it that we can never remove fields. My very
motivation for adding a header in the first place, was to give us
ability to extend this.

For a next round, I will include a Documentation file as you requested.
I could, for instance, explicitly mention that people parsing this
should first query the first line, which constitutes a header.

The main advantage I see in this approach, is that there is way less
data to be written using a header. Although your way works, it means we
will write the strings "nice", "system", etc. #cpu times. Quite a waste.

> 

  reply	other threads:[~2013-01-09 21:09 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-09 11:45 [PATCH v5 00/11] per-cgroup cpu-stat Glauber Costa
2013-01-09 11:45 ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 01/11] don't call cpuacct_charge in stop_task.c Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 02/11] cgroup: implement CFTYPE_NO_PREFIX Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 03/11] cgroup, sched: let cpu serve the same files as cpuacct Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-14  8:34   ` Sha Zhengju
2013-01-14  8:34     ` Sha Zhengju
2013-01-14 14:55     ` Glauber Costa
2013-01-14 14:55       ` Glauber Costa
2013-01-15 10:19       ` Sha Zhengju
2013-01-15 10:19         ` Sha Zhengju
2013-01-15 17:52         ` Glauber Costa
2013-01-15 17:52           ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 04/11] cgroup, sched: deprecate cpuacct Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 05/11] sched: adjust exec_clock to use it as cpu usage metric Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 06/11] cpuacct: don't actually do anything Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 07/11] account guest time per-cgroup as well Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 08/11] sched: Push put_prev_task() into pick_next_task() Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 09/11] record per-cgroup number of context switches Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 10/11] sched: change nr_context_switches calculation Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 11:45 ` [PATCH v5 11/11] sched: introduce cgroup file stat_percpu Glauber Costa
2013-01-09 11:45   ` Glauber Costa
2013-01-09 20:42   ` Andrew Morton
2013-01-09 20:42     ` Andrew Morton
2013-01-09 21:10     ` Glauber Costa [this message]
2013-01-09 21:10       ` Glauber Costa
2013-01-09 21:17       ` Andrew Morton
2013-01-09 21:17         ` Andrew Morton
2013-01-09 21:27         ` Glauber Costa
2013-01-09 21:27           ` Glauber Costa
2013-01-23 14:26           ` Glauber Costa
2013-01-23 14:26             ` Glauber Costa
2013-01-23 14:20     ` Glauber Costa
2013-01-23 14:20       ` Glauber Costa
2013-01-09 14:41 ` [PATCH v5 00/11] per-cgroup cpu-stat Tejun Heo
2013-01-09 14:41   ` Tejun Heo
2013-01-16  0:33 ` Colin Cross
2013-01-21 12:14   ` Glauber Costa
2013-01-21 12:14     ` Glauber Costa
2013-01-23  1:02     ` Tejun Heo
2013-01-23  1:02       ` Tejun Heo
2013-01-23  1:53       ` Colin Cross
2013-01-23  1:53         ` Colin Cross
2013-01-23  8:12         ` Glauber Costa
2013-01-23  8:12           ` Glauber Costa
2013-01-23 16:56         ` Tejun Heo
2013-01-23 16:56           ` Tejun Heo
2013-01-23 22:41           ` Colin Cross
2013-01-23 23:06             ` Tejun Heo
2013-01-23 23:06               ` Tejun Heo
2013-01-23 23:53               ` Colin Cross
2013-01-23 23:53                 ` Colin Cross

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50EDDCAA.6070004@parallels.com \
    --to=glommer@parallels.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pjt@google.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.