[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080405205914.GA25009@linux.vnet.ibm.com>
Date: Sun, 6 Apr 2008 02:29:14 +0530
From: Dhaval Giani <dhaval@...ux.vnet.ibm.com>
To: Balaji Rao <balajirrao@...il.com>
Cc: linux-kernel@...r.kernel.org, containers@...ts.osdl.org,
menage@...gle.com, a.p.zijlstra@...llo.nl, balbir@...ibm.com,
Srivatsa Vaddagiri <vatsa@...ux.vnet.ibm.com>
Subject: Re: [RFC][-mm] [1/2] Simple stats for cpu resource controller
On Sun, Apr 06, 2008 at 02:01:52AM +0530, Balaji Rao wrote:
> On Sunday 06 April 2008 01:10:41 am Dhaval Giani wrote:
> > > +};
> > > +
> > > +struct cpu_cgroup_stat_cpu {
> > > + s64 count[CPU_CGROUP_STAT_NSTATS];
> >
> > u64? time does not go negative :)
> Right. But these stats are not only going to measure time. We need the same
> variables for measuring other stats as well. I'm not sure if we would
> encounter scheduler stats that would count negative.
>
> Balbir, what do you say ?
I would prefer to keep the stats logically separate. So something like
struct cpu_cgroup_stat_cpu {
u64 time[];
s64 some_other_stat;
}
and so on. (I am not sure, is there some advantage gained by using
structs?) Makes the code more maintainable imho.
>
> > count also is not very clear? Can you give a more descriptive name?
> >
> ok. How does 'value' look ?
>
> <snip>
>
> > > +static s64 cpu_cgroup_read_stat(struct cpu_cgroup_stat *stat,
> > > + enum cpu_cgroup_stat_index idx)
> > > +{
> > > + int cpu;
> > > + s64 ret = 0;
> > > + unsigned long flags;
> >
> > > +
> > > + local_irq_save(flags);
> >
> > I am just wondering. Is local_irq_save() enough?
> >
> Hmmm.. You are right.This does not prevent concurrent updates on other CPUs
> from crossing a 32bit boundary. Am not sure how to do this in a safe way. I
> can only think of using atomic64_t now..
>
I am going to answer that one when I am awake :-)
--
regards,
Dhaval
--
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