Message ID | 20120822084312.17293.47596.stgit@ltc189.sdl.hitachi.co.jp |
---|---|
State | New |
Headers | show |
On Wed, 2012-08-22 at 17:43 +0900, Yoshihiro YUNOMAE wrote: > From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > > Count debugfs/tracing/per_cpu/cpu* to determine the > number of CPUs. I'm curious, do you find that sysconf doesn't return the # of CPUs the system has? I've had boxes where the per_cpu/cpu* had more cpus than the box actually holds. But this was a bug in the kernel, not the tool. This change log needs to have rational instead of just explaining what the patch does. Thanks, -- Steve > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Signed-off-by: Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com> > ---
(2012/08/22 22:41), Steven Rostedt wrote: > On Wed, 2012-08-22 at 17:43 +0900, Yoshihiro YUNOMAE wrote: >> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >> >> Count debugfs/tracing/per_cpu/cpu* to determine the >> number of CPUs. > > I'm curious, do you find that sysconf doesn't return the # of CPUs the > system has? No, sysconf returns the number of hosts CPUs, not guests. > I've had boxes where the per_cpu/cpu* had more cpus than the > box actually holds. But this was a bug in the kernel, not the tool. This > change log needs to have rational instead of just explaining what the > patch does. Ah, I see. Hmm, then this should be enabled by a command line option or an environment variable. Thank you,
(2012/08/23 11:01), Masami Hiramatsu wrote: > (2012/08/22 22:41), Steven Rostedt wrote: >> On Wed, 2012-08-22 at 17:43 +0900, Yoshihiro YUNOMAE wrote: >>> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >>> >>> Count debugfs/tracing/per_cpu/cpu* to determine the >>> number of CPUs. >> >> I'm curious, do you find that sysconf doesn't return the # of CPUs the >> system has? > > No, sysconf returns the number of hosts CPUs, not guests. > >> I've had boxes where the per_cpu/cpu* had more cpus than the >> box actually holds. But this was a bug in the kernel, not the tool. This >> change log needs to have rational instead of just explaining what the >> patch does. > > Ah, I see. Hmm, then this should be enabled by a command line > option or an environment variable. Oops, I misunderstood. I'll add more comment for why this should be tried instead of sysconf. Thank you,
On Thu, 2012-08-23 at 12:00 +0900, Masami Hiramatsu wrote: > (2012/08/23 11:01), Masami Hiramatsu wrote: > > (2012/08/22 22:41), Steven Rostedt wrote: > >> On Wed, 2012-08-22 at 17:43 +0900, Yoshihiro YUNOMAE wrote: > >>> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > >>> > >>> Count debugfs/tracing/per_cpu/cpu* to determine the > >>> number of CPUs. > >> > >> I'm curious, do you find that sysconf doesn't return the # of CPUs the > >> system has? > > > > No, sysconf returns the number of hosts CPUs, not guests. > > > >> I've had boxes where the per_cpu/cpu* had more cpus than the > >> box actually holds. But this was a bug in the kernel, not the tool. This > >> change log needs to have rational instead of just explaining what the > >> patch does. > > > > Ah, I see. Hmm, then this should be enabled by a command line > > option or an environment variable. > > Oops, I misunderstood. I'll add more comment for why this > should be tried instead of sysconf. And now that I understand why you are doing this, why not only do this if the TRACE_AGENT or DEBUG_TRACING_DIR is defined. That is, if we are doing it against a bare metal system, then sysconf should suffice, but if we are tracing against a guest, then it should use the tracing directory to determine the buffers. We could add options to override this, but I would think the default should just Do The Right Thing(tm). -- Steve
(2012/08/23 18:08), Steven Rostedt wrote: > On Thu, 2012-08-23 at 12:00 +0900, Masami Hiramatsu wrote: >> (2012/08/23 11:01), Masami Hiramatsu wrote: >>> (2012/08/22 22:41), Steven Rostedt wrote: >>>> On Wed, 2012-08-22 at 17:43 +0900, Yoshihiro YUNOMAE wrote: >>>>> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> >>>>> >>>>> Count debugfs/tracing/per_cpu/cpu* to determine the >>>>> number of CPUs. >>>> >>>> I'm curious, do you find that sysconf doesn't return the # of CPUs the >>>> system has? >>> >>> No, sysconf returns the number of hosts CPUs, not guests. >>> >>>> I've had boxes where the per_cpu/cpu* had more cpus than the >>>> box actually holds. But this was a bug in the kernel, not the tool. This >>>> change log needs to have rational instead of just explaining what the >>>> patch does. >>> >>> Ah, I see. Hmm, then this should be enabled by a command line >>> option or an environment variable. >> >> Oops, I misunderstood. I'll add more comment for why this >> should be tried instead of sysconf. > > And now that I understand why you are doing this, why not only do this > if the TRACE_AGENT or DEBUG_TRACING_DIR is defined. That is, if we are > doing it against a bare metal system, then sysconf should suffice, but > if we are tracing against a guest, then it should use the tracing > directory to determine the buffers. > > We could add options to override this, but I would think the default > should just Do The Right Thing(tm). Yeah, so I'd like to push this is the default method, and fix the kernel bug (but I'm not sure that is a bug). Thank you,
diff --git a/trace-record.c b/trace-record.c index 9dc18a9..ed18951 100644 --- a/trace-record.c +++ b/trace-record.c @@ -1179,6 +1179,41 @@ static void expand_event_list(void) } } +static int count_tracingdir_cpus(void) +{ + char *tracing_dir = NULL; + char *percpu_dir = NULL; + struct dirent **namelist; + int count = 0, n; + + /* Count cpus in per_cpu directory */ + tracing_dir = tracecmd_find_tracing_dir(); + if (!tracing_dir) + return 0; + percpu_dir = malloc_or_die(strlen(tracing_dir) + 9); + if (!percpu_dir) + goto err; + + sprintf(percpu_dir, "%s/per_cpu", tracing_dir); + + n = scandir(percpu_dir, &namelist, NULL, alphasort); + if (n > 0) { + while (n--) { + if (strncmp("cpu", namelist[n]->d_name, 3) == 0) + count++; + free(namelist[n]); + } + free(namelist); + } + + if (percpu_dir) + free(percpu_dir); +err: + if (tracing_dir) + free(tracing_dir); + return count; +} + static int count_cpus(void) { FILE *fp; @@ -1189,6 +1224,12 @@ static int count_cpus(void) size_t n; int r; + cpus = count_tracingdir_cpus(); + if (cpus > 0) + return cpus; + + warning("failed to use tracing_dir to determine number of CPUS"); + cpus = sysconf(_SC_NPROCESSORS_CONF); if (cpus > 0) return cpus;