Messages in this thread | | | Date | Mon, 23 Mar 2009 21:51:23 +0530 | From | Bharata B Rao <> | Subject | Re: [PATCH -tip] cpuacct: per-cgroup utime/stime statistics - v4 |
| |
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.
| |