* [PATCH v2 0/2] cpuacct: rename parameter in cpuusage_write for readability
@ 2016-03-04 9:47 Zhao Lei
2016-03-04 9:47 ` [PATCH v2 1/2] " Zhao Lei
2016-03-04 9:47 ` [PATCH v2 2/2] cpuacct: split usage into user_usage and sys_usage Zhao Lei
0 siblings, 2 replies; 6+ messages in thread
From: Zhao Lei @ 2016-03-04 9:47 UTC (permalink / raw)
To: zhaolei, cgroups, linux-kernel, mingo, peterz, tj
The name of 'reset' makes a little confusion in reading, we would
say, if we want to reset usage, return -EINVAL. That's not true.
Actually, we want to say, we only allow user to do a reset. This
patch rename reset to val and add a comment here, making the code
more readable.
Changelog v1->v2:
1: Rebase on top of 4.5-rc6
2: Fix little spelling typo in description.
3: Fix line over 80 characters
4: Add Acked-by: Tejun Heo <tj@kernel.org>
Yang Dongsheng (2):
cpuacct: rename parameter in cpuusage_write for readability
cpuacct: split usage into user_usage and sys_usage.
kernel/sched/cpuacct.c | 151 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 129 insertions(+), 22 deletions(-)
--
1.8.5.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] cpuacct: rename parameter in cpuusage_write for readability
2016-03-04 9:47 [PATCH v2 0/2] cpuacct: rename parameter in cpuusage_write for readability Zhao Lei
@ 2016-03-04 9:47 ` Zhao Lei
2016-03-11 17:42 ` Tejun Heo
2016-03-04 9:47 ` [PATCH v2 2/2] cpuacct: split usage into user_usage and sys_usage Zhao Lei
1 sibling, 1 reply; 6+ messages in thread
From: Zhao Lei @ 2016-03-04 9:47 UTC (permalink / raw)
To: zhaolei, cgroups, linux-kernel, mingo, peterz, tj; +Cc: Yang Dongsheng
From: Yang Dongsheng <yangds.fnst@cn.fujitsu.com>
The name of 'reset' makes a little confusion in reading, we would
say, if we want to reset usage, return -EINVAL. That's not true.
Actually, we want to say, we only allow user to do a reset. This
patch rename reset to val and add a comment here, making the code
more readable.
From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
kernel/sched/cpuacct.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index dd7cbb5..9c2bbf7 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -145,13 +145,16 @@ static u64 cpuusage_read(struct cgroup_subsys_state *css, struct cftype *cft)
}
static int cpuusage_write(struct cgroup_subsys_state *css, struct cftype *cft,
- u64 reset)
+ u64 val)
{
struct cpuacct *ca = css_ca(css);
int err = 0;
int i;
- if (reset) {
+ /*
+ * Only allow '0' here to do a reset.
+ */
+ if (val) {
err = -EINVAL;
goto out;
}
--
1.8.5.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] cpuacct: split usage into user_usage and sys_usage.
2016-03-04 9:47 [PATCH v2 0/2] cpuacct: rename parameter in cpuusage_write for readability Zhao Lei
2016-03-04 9:47 ` [PATCH v2 1/2] " Zhao Lei
@ 2016-03-04 9:47 ` Zhao Lei
2016-03-10 13:27 ` Peter Zijlstra
1 sibling, 1 reply; 6+ messages in thread
From: Zhao Lei @ 2016-03-04 9:47 UTC (permalink / raw)
To: zhaolei, cgroups, linux-kernel, mingo, peterz, tj; +Cc: Yang Dongsheng
From: Yang Dongsheng <yangds.fnst@cn.fujitsu.com>
Sometimes, cpuacct.usage is not detialed enough to user
to see how much usage a group used. We want to know how
much time it used in user mode and how much in kernel mode.
This patch introduce some more files to tell user these information.
# ls /sys/fs/cgroup/cpuacct/cpuacct.usage*
/sys/fs/cgroup/cpuacct/cpuacct.usage
/sys/fs/cgroup/cpuacct/cpuacct.usage_percpu
/sys/fs/cgroup/cpuacct/cpuacct.usage_user
/sys/fs/cgroup/cpuacct/cpuacct.usage_percpu_user
/sys/fs/cgroup/cpuacct/cpuacct.usage_sys
/sys/fs/cgroup/cpuacct/cpuacct.usage_percpu_sys
From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
---
kernel/sched/cpuacct.c | 144 ++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 124 insertions(+), 20 deletions(-)
diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index 9c2bbf7..f3446b8 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -25,11 +25,22 @@ enum cpuacct_stat_index {
CPUACCT_STAT_NSTATS,
};
+enum cpuacct_usage_index {
+ CPUACCT_USAGE_USER, /* ... user mode */
+ CPUACCT_USAGE_SYSTEM, /* ... kernel mode */
+
+ CPUACCT_USAGE_NRUSAGE,
+};
+
+struct cpuacct_usage {
+ u64 usages[CPUACCT_USAGE_NRUSAGE];
+};
+
/* track cpu usage of a group of tasks and its child groups */
struct cpuacct {
struct cgroup_subsys_state css;
/* cpuusage holds pointer to a u64-type object on every cpu */
- u64 __percpu *cpuusage;
+ struct cpuacct_usage __percpu *cpuusage;
struct kernel_cpustat __percpu *cpustat;
};
@@ -49,7 +60,7 @@ static inline struct cpuacct *parent_ca(struct cpuacct *ca)
return css_ca(ca->css.parent);
}
-static DEFINE_PER_CPU(u64, root_cpuacct_cpuusage);
+static DEFINE_PER_CPU(struct cpuacct_usage, root_cpuacct_cpuusage);
static struct cpuacct root_cpuacct = {
.cpustat = &kernel_cpustat,
.cpuusage = &root_cpuacct_cpuusage,
@@ -68,7 +79,7 @@ cpuacct_css_alloc(struct cgroup_subsys_state *parent_css)
if (!ca)
goto out;
- ca->cpuusage = alloc_percpu(u64);
+ ca->cpuusage = alloc_percpu(struct cpuacct_usage);
if (!ca->cpuusage)
goto out_free_ca;
@@ -96,54 +107,107 @@ static void cpuacct_css_free(struct cgroup_subsys_state *css)
kfree(ca);
}
-static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu)
+static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu,
+ enum cpuacct_usage_index index)
{
- u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
- u64 data;
+ struct cpuacct_usage *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+ u64 data = 0;
+ int i = 0;
+
+ /*
+ * We allow index == CPUACCT_USAGE_NRUSAGE here to read
+ * the sum of suages.
+ */
+ BUG_ON(index > CPUACCT_USAGE_NRUSAGE);
+
+ if (index == CPUACCT_USAGE_NRUSAGE) {
+ raw_spin_lock_irq(&cpu_rq(cpu)->lock);
+ for (i = 0; i < CPUACCT_USAGE_NRUSAGE; i++)
+ data += cpuusage->usages[i];
+ raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
+
+ goto out;
+ }
#ifndef CONFIG_64BIT
/*
* Take rq->lock to make 64-bit read safe on 32-bit platforms.
*/
raw_spin_lock_irq(&cpu_rq(cpu)->lock);
- data = *cpuusage;
+ data = cpuusage->usages[index];
raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
#else
- data = *cpuusage;
+ data = cpuusage->usages[index];
#endif
+out:
return data;
}
-static void cpuacct_cpuusage_write(struct cpuacct *ca, int cpu, u64 val)
+static void cpuacct_cpuusage_write(struct cpuacct *ca, int cpu,
+ enum cpuacct_usage_index index, u64 val)
{
- u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+ struct cpuacct_usage *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+ int i = 0;
+
+ /*
+ * We allow index == CPUACCT_USAGE_NRUSAGE here to write
+ * val to each index of usages.
+ */
+ BUG_ON(index > CPUACCT_USAGE_NRUSAGE);
+
+ if (index == CPUACCT_USAGE_NRUSAGE) {
+ raw_spin_lock_irq(&cpu_rq(cpu)->lock);
+ for (i = 0; i < CPUACCT_USAGE_NRUSAGE; i++)
+ cpuusage->usages[i] = val;
+ raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
+
+ return;
+ }
#ifndef CONFIG_64BIT
/*
* Take rq->lock to make 64-bit write safe on 32-bit platforms.
*/
raw_spin_lock_irq(&cpu_rq(cpu)->lock);
- *cpuusage = val;
+ cpuusage->usages[index] = val;
raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
#else
- *cpuusage = val;
+ cpuusage->usages[index] = val;
#endif
}
/* return total cpu usage (in nanoseconds) of a group */
-static u64 cpuusage_read(struct cgroup_subsys_state *css, struct cftype *cft)
+static u64 __cpuusage_read(struct cgroup_subsys_state *css,
+ enum cpuacct_usage_index index)
{
struct cpuacct *ca = css_ca(css);
u64 totalcpuusage = 0;
int i;
for_each_present_cpu(i)
- totalcpuusage += cpuacct_cpuusage_read(ca, i);
+ totalcpuusage += cpuacct_cpuusage_read(ca, i, index);
return totalcpuusage;
}
+static u64 cpuusage_user_read(struct cgroup_subsys_state *css,
+ struct cftype *cft)
+{
+ return __cpuusage_read(css, CPUACCT_USAGE_USER);
+}
+
+static u64 cpuusage_sys_read(struct cgroup_subsys_state *css,
+ struct cftype *cft)
+{
+ return __cpuusage_read(css, CPUACCT_USAGE_SYSTEM);
+}
+
+static u64 cpuusage_read(struct cgroup_subsys_state *css, struct cftype *cft)
+{
+ return __cpuusage_read(css, CPUACCT_USAGE_NRUSAGE);
+}
+
static int cpuusage_write(struct cgroup_subsys_state *css, struct cftype *cft,
u64 val)
{
@@ -159,27 +223,44 @@ static int cpuusage_write(struct cgroup_subsys_state *css, struct cftype *cft,
goto out;
}
- for_each_present_cpu(i)
- cpuacct_cpuusage_write(ca, i, 0);
+ for_each_present_cpu(i) {
+ cpuacct_cpuusage_write(ca, i, CPUACCT_USAGE_NRUSAGE, 0);
+ }
out:
return err;
}
-static int cpuacct_percpu_seq_show(struct seq_file *m, void *V)
+static int __cpuacct_percpu_seq_show(struct seq_file *m,
+ enum cpuacct_usage_index index)
{
struct cpuacct *ca = css_ca(seq_css(m));
u64 percpu;
int i;
for_each_present_cpu(i) {
- percpu = cpuacct_cpuusage_read(ca, i);
+ percpu = cpuacct_cpuusage_read(ca, i, index);
seq_printf(m, "%llu ", (unsigned long long) percpu);
}
seq_printf(m, "\n");
return 0;
}
+static int cpuacct_percpu_user_seq_show(struct seq_file *m, void *V)
+{
+ return __cpuacct_percpu_seq_show(m, CPUACCT_USAGE_USER);
+}
+
+static int cpuacct_percpu_sys_seq_show(struct seq_file *m, void *V)
+{
+ return __cpuacct_percpu_seq_show(m, CPUACCT_USAGE_SYSTEM);
+}
+
+static int cpuacct_percpu_seq_show(struct seq_file *m, void *V)
+{
+ return __cpuacct_percpu_seq_show(m, CPUACCT_USAGE_NRUSAGE);
+}
+
static const char * const cpuacct_stat_desc[] = {
[CPUACCT_STAT_USER] = "user",
[CPUACCT_STAT_SYSTEM] = "system",
@@ -220,10 +301,26 @@ static struct cftype files[] = {
.write_u64 = cpuusage_write,
},
{
+ .name = "usage_user",
+ .read_u64 = cpuusage_user_read,
+ },
+ {
+ .name = "usage_sys",
+ .read_u64 = cpuusage_sys_read,
+ },
+ {
.name = "usage_percpu",
.seq_show = cpuacct_percpu_seq_show,
},
{
+ .name = "usage_percpu_user",
+ .seq_show = cpuacct_percpu_user_seq_show,
+ },
+ {
+ .name = "usage_percpu_sys",
+ .seq_show = cpuacct_percpu_sys_seq_show,
+ },
+ {
.name = "stat",
.seq_show = cpuacct_stats_show,
},
@@ -239,6 +336,7 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
{
struct cpuacct *ca;
int cpu;
+ int user_time;
cpu = task_cpu(tsk);
@@ -246,9 +344,15 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
ca = task_ca(tsk);
+ user_time = user_mode(task_pt_regs(tsk));
+
while (true) {
- u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
- *cpuusage += cputime;
+ struct cpuacct_usage *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
+
+ if (user_time)
+ cpuusage->usages[CPUACCT_USAGE_USER] += cputime;
+ else
+ cpuusage->usages[CPUACCT_USAGE_SYSTEM] += cputime;
ca = parent_ca(ca);
if (!ca)
--
1.8.5.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] cpuacct: split usage into user_usage and sys_usage.
2016-03-04 9:47 ` [PATCH v2 2/2] cpuacct: split usage into user_usage and sys_usage Zhao Lei
@ 2016-03-10 13:27 ` Peter Zijlstra
2016-03-16 10:39 ` Zhao Lei
0 siblings, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2016-03-10 13:27 UTC (permalink / raw)
To: Zhao Lei; +Cc: cgroups, linux-kernel, mingo, tj, Yang Dongsheng
On Fri, Mar 04, 2016 at 05:47:06PM +0800, Zhao Lei wrote:
> +static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu,
> + enum cpuacct_usage_index index)
> {
> + struct cpuacct_usage *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> + u64 data = 0;
> + int i = 0;
> +
> + /*
> + * We allow index == CPUACCT_USAGE_NRUSAGE here to read
> + * the sum of suages.
> + */
> + BUG_ON(index > CPUACCT_USAGE_NRUSAGE);
> +
> + if (index == CPUACCT_USAGE_NRUSAGE) {
> + raw_spin_lock_irq(&cpu_rq(cpu)->lock);
> + for (i = 0; i < CPUACCT_USAGE_NRUSAGE; i++)
> + data += cpuusage->usages[i];
> + raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
Why do you unconditionally take the lock here? You really don't need it
on 64 bit.
> +
> + goto out;
> + }
>
> #ifndef CONFIG_64BIT
> /*
> * Take rq->lock to make 64-bit read safe on 32-bit platforms.
> */
> raw_spin_lock_irq(&cpu_rq(cpu)->lock);
> + data = cpuusage->usages[index];
> raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
> #else
> + data = cpuusage->usages[index];
> #endif
>
> +out:
> return data;
> }
>
> +static void cpuacct_cpuusage_write(struct cpuacct *ca, int cpu,
> + enum cpuacct_usage_index index, u64 val)
> {
> + struct cpuacct_usage *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> + int i = 0;
> +
> + /*
> + * We allow index == CPUACCT_USAGE_NRUSAGE here to write
> + * val to each index of usages.
> + */
> + BUG_ON(index > CPUACCT_USAGE_NRUSAGE);
> +
> + if (index == CPUACCT_USAGE_NRUSAGE) {
> + raw_spin_lock_irq(&cpu_rq(cpu)->lock);
> + for (i = 0; i < CPUACCT_USAGE_NRUSAGE; i++)
> + cpuusage->usages[i] = val;
> + raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
> +
> + return;
> + }
Same for the above, and the below is dead code, you only ever call this
with NRUSAGE.
> #ifndef CONFIG_64BIT
> /*
> * Take rq->lock to make 64-bit write safe on 32-bit platforms.
> */
> raw_spin_lock_irq(&cpu_rq(cpu)->lock);
> + cpuusage->usages[index] = val;
> raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
> #else
> + cpuusage->usages[index] = val;
> #endif
> }
>
> @@ -246,9 +344,15 @@ void cpuacct_charge(struct task_struct *tsk, u64 cputime)
>
> ca = task_ca(tsk);
>
> + user_time = user_mode(task_pt_regs(tsk));
> +
> while (true) {
> - u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> - *cpuusage += cputime;
> + struct cpuacct_usage *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> +
> + if (user_time)
> + cpuusage->usages[CPUACCT_USAGE_USER] += cputime;
> + else
> + cpuusage->usages[CPUACCT_USAGE_SYSTEM] += cputime;
>
> ca = parent_ca(ca);
> if (!ca)
Have you tried to measure the performance impact of this?
Also, that code seems particularly silly for not using this_cpu_ptr().
After all, we only ever call this on current.
Also that ca iteration looks daft, should we fix that to read:
for (ca = task_ca(tsk); ca; ca = parent_ca(ca))
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] cpuacct: rename parameter in cpuusage_write for readability
2016-03-04 9:47 ` [PATCH v2 1/2] " Zhao Lei
@ 2016-03-11 17:42 ` Tejun Heo
0 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2016-03-11 17:42 UTC (permalink / raw)
To: Zhao Lei; +Cc: cgroups, linux-kernel, mingo, peterz, Yang Dongsheng
On Fri, Mar 04, 2016 at 05:47:05PM +0800, Zhao Lei wrote:
> From: Yang Dongsheng <yangds.fnst@cn.fujitsu.com>
>
> The name of 'reset' makes a little confusion in reading, we would
> say, if we want to reset usage, return -EINVAL. That's not true.
>
> Actually, we want to say, we only allow user to do a reset. This
> patch rename reset to val and add a comment here, making the code
> more readable.
>
> From: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com>
Acked-by: Tejun Heo <tj@kernel.org>
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH v2 2/2] cpuacct: split usage into user_usage and sys_usage.
2016-03-10 13:27 ` Peter Zijlstra
@ 2016-03-16 10:39 ` Zhao Lei
0 siblings, 0 replies; 6+ messages in thread
From: Zhao Lei @ 2016-03-16 10:39 UTC (permalink / raw)
To: 'Peter Zijlstra'; +Cc: cgroups, linux-kernel, mingo, tj
Hi, Peter Zijlstra
Thanks for so detailed review.
> -----Original Message-----
> From: Peter Zijlstra [mailto:peterz@infradead.org]
> Sent: Thursday, March 10, 2016 9:27 PM
> To: Zhao Lei <zhaolei@cn.fujitsu.com>
> Cc: cgroups@vger.kernel.org; linux-kernel@vger.kernel.org;
> mingo@redhat.com; tj@kernel.org; Yang Dongsheng
> <yangds.fnst@cn.fujitsu.com>
> Subject: Re: [PATCH v2 2/2] cpuacct: split usage into user_usage and
> sys_usage.
>
> On Fri, Mar 04, 2016 at 05:47:06PM +0800, Zhao Lei wrote:
> > +static u64 cpuacct_cpuusage_read(struct cpuacct *ca, int cpu,
> > + enum cpuacct_usage_index index)
> > {
> > + struct cpuacct_usage *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> > + u64 data = 0;
> > + int i = 0;
> > +
> > + /*
> > + * We allow index == CPUACCT_USAGE_NRUSAGE here to read
> > + * the sum of suages.
> > + */
> > + BUG_ON(index > CPUACCT_USAGE_NRUSAGE);
> > +
> > + if (index == CPUACCT_USAGE_NRUSAGE) {
> > + raw_spin_lock_irq(&cpu_rq(cpu)->lock);
> > + for (i = 0; i < CPUACCT_USAGE_NRUSAGE; i++)
> > + data += cpuusage->usages[i];
> > + raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
>
> Why do you unconditionally take the lock here? You really don't need it
> on 64 bit.
>
Yes, will fix.
> > +
> > + goto out;
> > + }
> >
> > #ifndef CONFIG_64BIT
> > /*
> > * Take rq->lock to make 64-bit read safe on 32-bit platforms.
> > */
> > raw_spin_lock_irq(&cpu_rq(cpu)->lock);
> > + data = cpuusage->usages[index];
> > raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
> > #else
> > + data = cpuusage->usages[index];
> > #endif
> >
> > +out:
> > return data;
> > }
> >
> > +static void cpuacct_cpuusage_write(struct cpuacct *ca, int cpu,
> > + enum cpuacct_usage_index index, u64 val)
> > {
> > + struct cpuacct_usage *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> > + int i = 0;
> > +
> > + /*
> > + * We allow index == CPUACCT_USAGE_NRUSAGE here to write
> > + * val to each index of usages.
> > + */
> > + BUG_ON(index > CPUACCT_USAGE_NRUSAGE);
> > +
> > + if (index == CPUACCT_USAGE_NRUSAGE) {
> > + raw_spin_lock_irq(&cpu_rq(cpu)->lock);
> > + for (i = 0; i < CPUACCT_USAGE_NRUSAGE; i++)
> > + cpuusage->usages[i] = val;
> > + raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
> > +
> > + return;
> > + }
>
> Same for the above, and the below is dead code, you only ever call this
> with NRUSAGE.
>
Good point.
> > #ifndef CONFIG_64BIT
> > /*
> > * Take rq->lock to make 64-bit write safe on 32-bit platforms.
> > */
> > raw_spin_lock_irq(&cpu_rq(cpu)->lock);
> > + cpuusage->usages[index] = val;
> > raw_spin_unlock_irq(&cpu_rq(cpu)->lock);
> > #else
> > + cpuusage->usages[index] = val;
> > #endif
> > }
> >
>
> > @@ -246,9 +344,15 @@ void cpuacct_charge(struct task_struct *tsk, u64
> cputime)
> >
> > ca = task_ca(tsk);
> >
> > + user_time = user_mode(task_pt_regs(tsk));
> > +
> > while (true) {
> > - u64 *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> > - *cpuusage += cputime;
> > + struct cpuacct_usage *cpuusage = per_cpu_ptr(ca->cpuusage, cpu);
> > +
> > + if (user_time)
> > + cpuusage->usages[CPUACCT_USAGE_USER] += cputime;
> > + else
> > + cpuusage->usages[CPUACCT_USAGE_SYSTEM] += cputime;
> >
> > ca = parent_ca(ca);
> > if (!ca)
>
> Have you tried to measure the performance impact of this?
>
> Also, that code seems particularly silly for not using this_cpu_ptr().
> After all, we only ever call this on current.
>
> Also that ca iteration looks daft, should we fix that to read:
>
> for (ca = task_ca(tsk); ca; ca = parent_ca(ca))
I'll rewrite this code block.
Thanks
Zhaolei
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-16 10:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-04 9:47 [PATCH v2 0/2] cpuacct: rename parameter in cpuusage_write for readability Zhao Lei
2016-03-04 9:47 ` [PATCH v2 1/2] " Zhao Lei
2016-03-11 17:42 ` Tejun Heo
2016-03-04 9:47 ` [PATCH v2 2/2] cpuacct: split usage into user_usage and sys_usage Zhao Lei
2016-03-10 13:27 ` Peter Zijlstra
2016-03-16 10:39 ` Zhao Lei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).