Date: Mon, 23 Mar 2009 21:51:23 +0530
From: Bharata B Rao <bharata@linux.vnet.ibm.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, Balaji Rao <balajirrao@gmail.com>,
       Dhaval Giani <dhaval@linux.vnet.ibm.com>,
       Balbir Singh <balbir@linux.vnet.ibm.com>,
       Li Zefan <lizf@cn.fujitsu.com>, Paul Menage <menage@google.com>,
       Ingo Molnar <mingo@elte.hu>, Peter Zijlstra <a.p.zijlstra@chello.nl>,
       KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Subject: Re: [PATCH -tip] cpuacct: per-cgroup utime/stime statistics - v4
Message-ID: <20090323162123.GA3858@in.ibm.com>
Reply-To: bharata@linux.vnet.ibm.com
References: <20090323043538.GB3306@in.ibm.com> <20090323042123.87a673ac.akpm@linux-foundation.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20090323042123.87a673ac.akpm@linux-foundation.org>
User-Agent: Mutt/1.5.18 (2008-05-17)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1344
Lines: 35

On Mon, Mar 23, 2009 at 04:21:23AM -0700, Andrew Morton wrote:
> On Mon, 23 Mar 2009 10:05:38 +0530 Bharata B Rao <bharata@linux.vnet.ibm.com> wrote:
> 
> > +static int cpuacct_stats_show(struct cgroup *cgrp, struct cftype *cft,
> > +		struct cgroup_map_cb *cb)
> > +{
> > +	struct cpuacct *ca = cgroup_ca(cgrp);
> > +	int i;
> > +
> > +	for (i = 0; i < CPUACCT_STAT_NSTATS; i++) {
> > +		s64 val = percpu_counter_read(&ca->cpustat[i]);
> > +		val = cputime_to_clock_t(val);
> > +		cb->fill(cb, cpuacct_stat_desc[i], val);
> > +	}
> > +	return 0;
> > +}
> 
> I'd have expected `val' to have type clock_t.  But clock_t is 32-bit on
> 32-bit x86.
> 
> Is it correct to pass a 64-bit value to a function which takes a 32-bit
> value and to then copy the 32-bit return value into a 64-bit variable?

Yes, doesn't look neat. Since the accumulated stat(val here) is of 64bit type,
I could use cputime64_to_clock_t(val) in the above code. Storing the resulting
32bit clock_t in a 64 bit variable(which will be used by cb->fill) should be
fine I believe. Let me know if I got this right.

Regards,
Bharata.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/