pcp
[Top] [All Lists]

Re: [PATCH] Fix Linux PMDA CPU time metrics

To: Nathan Scott <nscott@xxxxxxxxxx>
Subject: Re: [PATCH] Fix Linux PMDA CPU time metrics
From: Michael Newton <kimbrr@xxxxxxxxxxxxxxxxx>
Date: Thu, 8 Feb 2007 12:10:35 +1100
Cc: Mark Goodwin <markgw@xxxxxxx>, pcp@xxxxxxxxxxx, Daniel Moore <dxm@xxxxxxxxxxxxxxxxx>
In-reply-to: <1164090739.4695.301.camel@edge>
References: <1164090739.4695.301.camel@edge>
Sender: pcp-bounce@xxxxxxxxxxx
On Tue, 21 Nov 2006, Nathan Scott wrote:
> This patch fixes the precision of the allcpu and percpu millisecond
> counters.  This has been the source of problems in the past, and it
> looks like we missed another size transition awhile back in 2.6.5+.
>
> Historically, 2.4 and earlier kernels used a funky int/long mix for
> idle and other CPU times.  Some code was put in to try to circumvent
> counter wrapping, in the agent, back then (hohum).  Then, in 2.6.0,
> though 2.6.4 (inclusive) all CPU time counters were made 32 bits for
> all platforms (argh!).  Then, in 2.6.5 (and beyond) _all_ CPU time
> counters got changed to be 64 bits unconditionally.
>
> At least someone finally got it right. :]  However, we missed this
> last transition to 64 bits, and the Linux agent hasn't been updated.
> This patch does that, and dynamically sets the type of the CPU time
> metrics depending on the 3 kernel versions/flavours.


+    if (sscanf(kernel_uname.release, "%d.%d", &major, &minor) == 2) {
+       if (major < 2 || (major == 2 && minor <= 4)) {  /* 2.4 and earlier
*/
+            fprintf(stderr, "NOTICE: using kernel 2.4 or earlier CPU types\n");
+           _pm_ctxt_size = 4;
+           _pm_intr_size = 4;
+           _pm_cputime_size = 4;
+           _pm_idletime_size = sizeof(unsigned long);
+       }
+       else if (major == 2 && minor >= 0 && minor <= 4) {  /* 2.6.0->.4
*/
+            fprintf(stderr, "NOTICE: using kernel 2.6.0 to 2.6.4 CPU types\n");
+           _pm_cputime_size = 4;
+           _pm_idletime_size = 4;
+       }
+       else
+           fprintf(stderr, "NOTICE: using 64 bit CPU time types\n");
+    }

Looks to me like your "else if" clause will never happen.. aren't you
missing a variable to contain the 3rd element of the release number
(major.minor.missing) ? Only thing im not sure on is, did linux
kernel release nums always have 3 components, or do we have to handle the
possibility that earlier ones may have only 2?

PS: im not sure if you got the news Daniel is now my (immediate) boss

<Prev in Thread] Current Thread [Next in Thread>