Re: [PATCH v2 1/7] perf tools: Move parse_clockid into clockid object

[Arnaldo Carvalho de Melo]

Em Wed, Aug 05, 2020 at 11:34:38AM +0200, Jiri Olsa escreveu:
> Move parse_clockid and all needed clcckid related stuff
> into clockid object. We are going to add clockid_name
> function in following change, so it's better it's placed
> in separated object and not in builtin-record.c.
> 
> No functional change is intended.


Thanks, applied.

- Arnaldo
 
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
>  tools/perf/builtin-record.c |  98 +-------------------------------
>  tools/perf/util/Build       |   1 +
>  tools/perf/util/clockid.c   | 108 ++++++++++++++++++++++++++++++++++++
>  tools/perf/util/clockid.h   |   9 +++
>  4 files changed, 119 insertions(+), 97 deletions(-)
>  create mode 100644 tools/perf/util/clockid.c
>  create mode 100644 tools/perf/util/clockid.h
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index b6bdccd875bc..eb42d2d9d16b 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -46,6 +46,7 @@
>  #include "util/bpf-event.h"
>  #include "util/util.h"
>  #include "util/pfm.h"
> +#include "util/clockid.h"
>  #include "asm/bug.h"
>  #include "perf.h"
>  
> @@ -2078,103 +2079,6 @@ static int perf_record_config(const char *var, const char *value, void *cb)
>  	return 0;
>  }
>  
> -struct clockid_map {
> -	const char *name;
> -	int clockid;
> -};
> -
> -#define CLOCKID_MAP(n, c)	\
> -	{ .name = n, .clockid = (c), }
> -
> -#define CLOCKID_END	{ .name = NULL, }
> -
> -
> -/*
> - * Add the missing ones, we need to build on many distros...
> - */
> -#ifndef CLOCK_MONOTONIC_RAW
> -#define CLOCK_MONOTONIC_RAW 4
> -#endif
> -#ifndef CLOCK_BOOTTIME
> -#define CLOCK_BOOTTIME 7
> -#endif
> -#ifndef CLOCK_TAI
> -#define CLOCK_TAI 11
> -#endif
> -
> -static const struct clockid_map clockids[] = {
> -	/* available for all events, NMI safe */
> -	CLOCKID_MAP("monotonic", CLOCK_MONOTONIC),
> -	CLOCKID_MAP("monotonic_raw", CLOCK_MONOTONIC_RAW),
> -
> -	/* available for some events */
> -	CLOCKID_MAP("realtime", CLOCK_REALTIME),
> -	CLOCKID_MAP("boottime", CLOCK_BOOTTIME),
> -	CLOCKID_MAP("tai", CLOCK_TAI),
> -
> -	/* available for the lazy */
> -	CLOCKID_MAP("mono", CLOCK_MONOTONIC),
> -	CLOCKID_MAP("raw", CLOCK_MONOTONIC_RAW),
> -	CLOCKID_MAP("real", CLOCK_REALTIME),
> -	CLOCKID_MAP("boot", CLOCK_BOOTTIME),
> -
> -	CLOCKID_END,
> -};
> -
> -static int get_clockid_res(clockid_t clk_id, u64 *res_ns)
> -{
> -	struct timespec res;
> -
> -	*res_ns = 0;
> -	if (!clock_getres(clk_id, &res))
> -		*res_ns = res.tv_nsec + res.tv_sec * NSEC_PER_SEC;
> -	else
> -		pr_warning("WARNING: Failed to determine specified clock resolution.\n");
> -
> -	return 0;
> -}
> -
> -static int parse_clockid(const struct option *opt, const char *str, int unset)
> -{
> -	struct record_opts *opts = (struct record_opts *)opt->value;
> -	const struct clockid_map *cm;
> -	const char *ostr = str;
> -
> -	if (unset) {
> -		opts->use_clockid = 0;
> -		return 0;
> -	}
> -
> -	/* no arg passed */
> -	if (!str)
> -		return 0;
> -
> -	/* no setting it twice */
> -	if (opts->use_clockid)
> -		return -1;
> -
> -	opts->use_clockid = true;
> -
> -	/* if its a number, we're done */
> -	if (sscanf(str, "%d", &opts->clockid) == 1)
> -		return get_clockid_res(opts->clockid, &opts->clockid_res_ns);
> -
> -	/* allow a "CLOCK_" prefix to the name */
> -	if (!strncasecmp(str, "CLOCK_", 6))
> -		str += 6;
> -
> -	for (cm = clockids; cm->name; cm++) {
> -		if (!strcasecmp(str, cm->name)) {
> -			opts->clockid = cm->clockid;
> -			return get_clockid_res(opts->clockid,
> -					       &opts->clockid_res_ns);
> -		}
> -	}
> -
> -	opts->use_clockid = false;
> -	ui__warning("unknown clockid %s, check man page\n", ostr);
> -	return -1;
> -}
>  
>  static int record__parse_affinity(const struct option *opt, const char *str, int unset)
>  {
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 380e6a9f564d..494626e303f5 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -128,6 +128,7 @@ perf-y += expr-bison.o
>  perf-y += expr.o
>  perf-y += branch.o
>  perf-y += mem2node.o
> +perf-y += clockid.o
>  
>  perf-$(CONFIG_LIBBPF) += bpf-loader.o
>  perf-$(CONFIG_LIBBPF) += bpf_map.o
> diff --git a/tools/perf/util/clockid.c b/tools/perf/util/clockid.c
> new file mode 100644
> index 000000000000..b7a08606dc3e
> --- /dev/null
> +++ b/tools/perf/util/clockid.c
> @@ -0,0 +1,108 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <subcmd/parse-options.h>
> +#include <stdio.h>
> +#include <time.h>
> +#include <strings.h>
> +#include <linux/time64.h>
> +#include "debug.h"
> +#include "clockid.h"
> +#include "record.h"
> +
> +struct clockid_map {
> +	const char *name;
> +	int clockid;
> +};
> +
> +#define CLOCKID_MAP(n, c)	\
> +	{ .name = n, .clockid = (c), }
> +
> +#define CLOCKID_END	{ .name = NULL, }
> +
> +
> +/*
> + * Add the missing ones, we need to build on many distros...
> + */
> +#ifndef CLOCK_MONOTONIC_RAW
> +#define CLOCK_MONOTONIC_RAW 4
> +#endif
> +#ifndef CLOCK_BOOTTIME
> +#define CLOCK_BOOTTIME 7
> +#endif
> +#ifndef CLOCK_TAI
> +#define CLOCK_TAI 11
> +#endif
> +
> +static const struct clockid_map clockids[] = {
> +	/* available for all events, NMI safe */
> +	CLOCKID_MAP("monotonic", CLOCK_MONOTONIC),
> +	CLOCKID_MAP("monotonic_raw", CLOCK_MONOTONIC_RAW),
> +
> +	/* available for some events */
> +	CLOCKID_MAP("realtime", CLOCK_REALTIME),
> +	CLOCKID_MAP("boottime", CLOCK_BOOTTIME),
> +	CLOCKID_MAP("tai", CLOCK_TAI),
> +
> +	/* available for the lazy */
> +	CLOCKID_MAP("mono", CLOCK_MONOTONIC),
> +	CLOCKID_MAP("raw", CLOCK_MONOTONIC_RAW),
> +	CLOCKID_MAP("real", CLOCK_REALTIME),
> +	CLOCKID_MAP("boot", CLOCK_BOOTTIME),
> +
> +	CLOCKID_END,
> +};
> +
> +static int get_clockid_res(clockid_t clk_id, u64 *res_ns)
> +{
> +	struct timespec res;
> +
> +	*res_ns = 0;
> +	if (!clock_getres(clk_id, &res))
> +		*res_ns = res.tv_nsec + res.tv_sec * NSEC_PER_SEC;
> +	else
> +		pr_warning("WARNING: Failed to determine specified clock resolution.\n");
> +
> +	return 0;
> +}
> +
> +int parse_clockid(const struct option *opt, const char *str, int unset)
> +{
> +	struct record_opts *opts = (struct record_opts *)opt->value;
> +	const struct clockid_map *cm;
> +	const char *ostr = str;
> +
> +	if (unset) {
> +		opts->use_clockid = 0;
> +		return 0;
> +	}
> +
> +	/* no arg passed */
> +	if (!str)
> +		return 0;
> +
> +	/* no setting it twice */
> +	if (opts->use_clockid)
> +		return -1;
> +
> +	opts->use_clockid = true;
> +
> +	/* if its a number, we're done */
> +	if (sscanf(str, "%d", &opts->clockid) == 1)
> +		return get_clockid_res(opts->clockid, &opts->clockid_res_ns);
> +
> +	/* allow a "CLOCK_" prefix to the name */
> +	if (!strncasecmp(str, "CLOCK_", 6))
> +		str += 6;
> +
> +	for (cm = clockids; cm->name; cm++) {
> +		if (!strcasecmp(str, cm->name)) {
> +			opts->clockid = cm->clockid;
> +			return get_clockid_res(opts->clockid,
> +					       &opts->clockid_res_ns);
> +		}
> +	}
> +
> +	opts->use_clockid = false;
> +	ui__warning("unknown clockid %s, check man page\n", ostr);
> +	return -1;
> +}
> diff --git a/tools/perf/util/clockid.h b/tools/perf/util/clockid.h
> new file mode 100644
> index 000000000000..8e567b3ebbbd
> --- /dev/null
> +++ b/tools/perf/util/clockid.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __PERF_CLOCKID_H
> +#define __PERF_CLOCKID_H
> +
> +struct option;
> +int parse_clockid(const struct option *opt, const char *str, int unset);
> +
> +#endif
> -- 
> 2.25.4
> 

-- 

- Arnaldo