Re: [PATCH v17 05/18] trace-cmd: Add new library API for local CPU count

[Steven Rostedt <rostedt@goodmis.org>]

On Tue,  3 Dec 2019 12:35:09 +0200
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> In order to reuse the code, the function detecting the
> number of local CPU is moved from trace-cmd application to
> libtracecmd. The following new library API is introduced:
> 	int tracecmd_count_cpus(void);
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  include/trace-cmd/trace-cmd.h  |  3 +++
>  lib/trace-cmd/trace-util.c     | 40 +++++++++++++++++++++++++++++
>  tracecmd/include/trace-local.h |  2 --
>  tracecmd/trace-agent.c         |  2 +-
>  tracecmd/trace-profile.c       |  2 +-
>  tracecmd/trace-record.c        | 46 +++-------------------------------
>  tracecmd/trace-stat.c          |  4 +--
>  7 files changed, 50 insertions(+), 49 deletions(-)
> 
> diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h
> index 5287d23..5d4292e 100644
> --- a/include/trace-cmd/trace-cmd.h
> +++ b/include/trace-cmd/trace-cmd.h
> @@ -427,6 +427,9 @@ void tracecmd_plog(const char *fmt, ...);
>  void tracecmd_plog_error(const char *fmt, ...);
>  int tracecmd_set_logfile(char *logfile);
>  
> +/* --- System --- */
> +int tracecmd_count_cpus(void);
> +
>  /* --- Hack! --- */
>  int tracecmd_blk_hack(struct tracecmd_input *handle);
>  
> diff --git a/lib/trace-cmd/trace-util.c b/lib/trace-cmd/trace-util.c
> index 4c70d9a..e019dce 100644
> --- a/lib/trace-cmd/trace-util.c
> +++ b/lib/trace-cmd/trace-util.c
> @@ -1112,6 +1112,46 @@ int tracecmd_stack_tracer_status(int *status)
>  	return 1; /* full success */
>  }
>  
> +int tracecmd_count_cpus(void)
> +{
> +	FILE *fp;
> +	char buf[1024];
> +	int cpus = 0;
> +	char *pbuf;
> +	size_t *pn;
> +	size_t n;
> +	int r;
> +
> +	cpus = sysconf(_SC_NPROCESSORS_CONF);
> +	if (cpus > 0)
> +		return cpus;
> +
> +	warning("sysconf could not determine number of CPUS");

As this is now a library function, probably should have a static
variable here:

	static int once;

	if (!once) {
		once++;
		warning("...");
	}

No need to do more than one warning.


> +
> +	/* Do the hack to figure out # of CPUS */
> +	n = 1024;
> +	pn = &n;
> +	pbuf = buf;
> +
> +	fp = fopen("/proc/cpuinfo", "r");
> +	if (!fp)
> +		die("Can not read cpuinfo");

Need to get rid of the die(). Could be in a separate patch.

Will also need to have the call sites check to see if the returned
number is negative, and fail there.

-- Steve

> +
> +	while ((r = getline(&pbuf, pn, fp)) >= 0) {
> +		char *p;
> +
> +		if (strncmp(buf, "processor", 9) != 0)
> +			continue;
> +		for (p = buf+9; isspace(*p); p++)
> +			;
> +		if (*p == ':')
> +			cpus++;
> +	}
> +	fclose(fp);
> +
> +	return cpus;
> +}
> +
>  /*
>   * tracecmd_write_file - Write in trace file
>   * @file: Full name of the trace file.
> diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h