Currently the schedstats implementation does not allow the statistics
to be reset. This patch aims to allow that.
echo 0 > cpuacct.usage
resets the usage. Any other value is not allowed and returns -EINVAL.
Signed-off-by: Dhaval Giani <[email protected]>
---
kernel/sched.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+)
Index: linux-2.6/kernel/sched.c
===================================================================
--- linux-2.6.orig/kernel/sched.c 2008-02-28 20:05:27.000000000 +0530
+++ linux-2.6/kernel/sched.c 2008-02-28 20:05:30.000000000 +0530
@@ -8243,10 +8243,34 @@ static u64 cpuusage_read(struct cgroup *
return totalcpuusage;
}
+static int cpuusage_write(struct cgroup *cgrp, struct cftype *cftype,
+ u64 reset)
+{
+ struct cpuacct *ca = cgroup_ca(cgrp);
+ int err = 0;
+ int i;
+
+ if (reset) {
+ err = -EINVAL;
+ goto out;
+ }
+
+ for_each_possible_cpu(i) {
+ u64 *cpuusage = percpu_ptr(ca->cpuusage, i);
+
+ spin_lock_irq(&cpu_rq(i)->lock);
+ *cpuusage = 0;
+ spin_unlock_irq(&cpu_rq(i)->lock);
+ }
+out:
+ return err;
+}
+
static struct cftype files[] = {
{
.name = "usage",
.read_uint = cpuusage_read,
+ .write_uint = cpuusage_write,
},
};
--
regards,
Dhaval
On Thu, Feb 28, 2008 at 8:32 PM, Dhaval Giani <[email protected]> wrote:
> Currently the schedstats implementation does not allow the statistics
> to be reset. This patch aims to allow that.
>
> echo 0 > cpuacct.usage
>
> resets the usage. Any other value is not allowed and returns -EINVAL.
>
> Signed-off-by: Dhaval Giani <[email protected]>
>
> ---
> kernel/sched.c | 24 ++++++++++++++++++++++++
> 1 files changed, 24 insertions(+)
>
> Index: linux-2.6/kernel/sched.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched.c 2008-02-28 20:05:27.000000000 +0530
> +++ linux-2.6/kernel/sched.c 2008-02-28 20:05:30.000000000 +0530
> @@ -8243,10 +8243,34 @@ static u64 cpuusage_read(struct cgroup *
> return totalcpuusage;
> }
>
> +static int cpuusage_write(struct cgroup *cgrp, struct cftype *cftype,
> + u64 reset)
> +{
> + struct cpuacct *ca = cgroup_ca(cgrp);
> + int err = 0;
> + int i;
> +
> + if (reset) {
> + err = -EINVAL;
> + goto out;
> + }
> +
> + for_each_possible_cpu(i) {
> + u64 *cpuusage = percpu_ptr(ca->cpuusage, i);
> +
> + spin_lock_irq(&cpu_rq(i)->lock);
> + *cpuusage = 0;
> + spin_unlock_irq(&cpu_rq(i)->lock);
> + }
> +out:
> + return err;
> +}
> +
> static struct cftype files[] = {
> {
> .name = "usage",
> .read_uint = cpuusage_read,
> + .write_uint = cpuusage_write,
> },
> };
>
Can I suggest, in order to be more generic, that this patch instead
set each CPU's counter to the written value divided by the number of
CPUs? (Either forgetting the remainder, or spreading it among the
first few CPUs).
Paul
On Thu, Feb 28, 2008 at 09:48:51PM -0800, Paul Menage wrote:
> Can I suggest, in order to be more generic, that this patch instead
> set each CPU's counter to the written value divided by the number of
> CPUs? (Either forgetting the remainder, or spreading it among the
> first few CPUs).
Paul,
When would you want to reset the counters to non-zero value?
What would be its usecase?
--
Regards,
vatsa
On Thu, Feb 28, 2008 at 09:48:51PM -0800, Paul Menage wrote:
> On Thu, Feb 28, 2008 at 8:32 PM, Dhaval Giani <[email protected]> wrote:
>
> Can I suggest, in order to be more generic, that this patch instead
> set each CPU's counter to the written value divided by the number of
> CPUs? (Either forgetting the remainder, or spreading it among the
> first few CPUs).
>
This patch is only allowing a reset of the stats. In what sort of a
situation would one be looking for changing the usage value?
thanks,
--
regards,
Dhaval
On Thu, Feb 28, 2008 at 10:04 PM, Dhaval Giani
<[email protected]> wrote:
> On Thu, Feb 28, 2008 at 09:48:51PM -0800, Paul Menage wrote:
>
> > On Thu, Feb 28, 2008 at 8:32 PM, Dhaval Giani <[email protected]> wrote:
> >
>
> > Can I suggest, in order to be more generic, that this patch instead
> > set each CPU's counter to the written value divided by the number of
> > CPUs? (Either forgetting the remainder, or spreading it among the
> > first few CPUs).
> >
>
> This patch is only allowing a reset of the stats. In what sort of a
> situation would one be looking for changing the usage value?
How about Checkpoint/Restart?
It seems that if you're going to the effort to notice a non-zero value
and return an error, it wouldn't be much more effort to do something
useful with that non-zero value instead.
Paul
On Fri, 2008-02-29 at 02:08 -0800, Paul Menage wrote:
> On Thu, Feb 28, 2008 at 10:04 PM, Dhaval Giani
> <[email protected]> wrote:
> > On Thu, Feb 28, 2008 at 09:48:51PM -0800, Paul Menage wrote:
> >
> > > On Thu, Feb 28, 2008 at 8:32 PM, Dhaval Giani <[email protected]> wrote:
> > >
> >
> > > Can I suggest, in order to be more generic, that this patch instead
> > > set each CPU's counter to the written value divided by the number of
> > > CPUs? (Either forgetting the remainder, or spreading it among the
> > > first few CPUs).
> > >
> >
> > This patch is only allowing a reset of the stats. In what sort of a
> > situation would one be looking for changing the usage value?
>
> How about Checkpoint/Restart?
Shouldn't that be a kernel side thing? And from the kernel you can put
back any value you fancy.
* Dhaval Giani <[email protected]> [2008-02-29 10:02:44]:
> Currently the schedstats implementation does not allow the statistics
> to be reset. This patch aims to allow that.
>
> echo 0 > cpuacct.usage
>
> resets the usage. Any other value is not allowed and returns -EINVAL.
>
> Signed-off-by: Dhaval Giani <[email protected]>
>
> ---
> kernel/sched.c | 24 ++++++++++++++++++++++++
> 1 files changed, 24 insertions(+)
>
> Index: linux-2.6/kernel/sched.c
> ===================================================================
> --- linux-2.6.orig/kernel/sched.c 2008-02-28 20:05:27.000000000 +0530
> +++ linux-2.6/kernel/sched.c 2008-02-28 20:05:30.000000000 +0530
> @@ -8243,10 +8243,34 @@ static u64 cpuusage_read(struct cgroup *
> return totalcpuusage;
> }
>
> +static int cpuusage_write(struct cgroup *cgrp, struct cftype *cftype,
> + u64 reset)
> +{
> + struct cpuacct *ca = cgroup_ca(cgrp);
> + int err = 0;
> + int i;
> +
> + if (reset) {
> + err = -EINVAL;
> + goto out;
> + }
> +
Why not use
if (reset)
return -EINVAL;
> + for_each_possible_cpu(i) {
> + u64 *cpuusage = percpu_ptr(ca->cpuusage, i);
> +
> + spin_lock_irq(&cpu_rq(i)->lock);
> + *cpuusage = 0;
> + spin_unlock_irq(&cpu_rq(i)->lock);
> + }
> +out:
> + return err;
> +}
> +
> static struct cftype files[] = {
> {
> .name = "usage",
> .read_uint = cpuusage_read,
> + .write_uint = cpuusage_write,
> },
> };
>
>
Looks good
Acked-by: Balbir Singh <[email protected]>
--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL
* Dhaval Giani <[email protected]> wrote:
> Currently the schedstats implementation does not allow the statistics
> to be reset. This patch aims to allow that.
>
> echo 0 > cpuacct.usage
>
> resets the usage. Any other value is not allowed and returns -EINVAL.
i've applied this for now, please send any updates that might come out
of the interface discussions.
Ingo