Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1530549 > unrolled thread

[PATCH 1/6] perf tool: Add time-based utility functions

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/6] perf tool: Add time-based utility functions David Ahern <dsa@cumulusnetworks.com> - 2016-11-25 22:50 +0100
    Re: [PATCH 1/6] perf tool: Add time-based utility functions Jiri Olsa <jolsa@redhat.com> - 2016-11-28 15:00 +0100
    Re: [PATCH 1/6] perf tool: Add time-based utility functions Jiri Olsa <jolsa@redhat.com> - 2016-11-28 15:00 +0100
      Re: [PATCH 1/6] perf tool: Add time-based utility functions David Ahern <dsahern@gmail.com> - 2016-11-28 18:30 +0100
        Re: [PATCH 1/6] perf tool: Add time-based utility functions David Ahern <dsahern@gmail.com> - 2016-11-29 17:10 +0100
        Re: [PATCH 1/6] perf tool: Add time-based utility functions Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-11-29 17:10 +0100
    Re: [PATCH 1/6] perf tool: Add time-based utility functions Jiri Olsa <jolsa@redhat.com> - 2016-11-28 15:00 +0100
      Re: [PATCH 1/6] perf tool: Add time-based utility functions David Ahern <dsahern@gmail.com> - 2016-11-28 18:40 +0100
        Re: [PATCH 1/6] perf tool: Add time-based utility functions Jiri Olsa <jolsa@redhat.com> - 2016-11-28 19:20 +0100
          Re: [PATCH 1/6] perf tool: Add time-based utility functions David Ahern <dsahern@gmail.com> - 2016-11-28 19:20 +0100

#1530549 — [PATCH 1/6] perf tool: Add time-based utility functions

From: David Ahern <dsahern@gmail.com>

Add function to parse a user time string of the form <start>,<stop>
where start and stop are time in sec.nsec format. Both start and stop
times are optional.

Add function to determine if a sample time is within a given time
time window of interest.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 tools/perf/util/Build        |  1 +
 tools/perf/util/time-utils.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/time-utils.h | 12 +++++++
 3 files changed, 98 insertions(+)
 create mode 100644 tools/perf/util/time-utils.c
 create mode 100644 tools/perf/util/time-utils.h

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 1dc67efad634..78f139978e7a 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -87,6 +87,7 @@ libperf-y += help-unknown-cmd.o
 libperf-y += mem-events.o
 libperf-y += vsprintf.o
 libperf-y += drv_configs.o
+libperf-y += time-utils.o
 
 libperf-$(CONFIG_LIBBPF) += bpf-loader.o
 libperf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o
diff --git a/tools/perf/util/time-utils.c b/tools/perf/util/time-utils.c
new file mode 100644
index 000000000000..e584aeae9834
--- /dev/null
+++ b/tools/perf/util/time-utils.c
@@ -0,0 +1,85 @@
+#include <string.h>
+#include <sys/time.h>
+#include <time.h>
+#include <errno.h>
+#include <inttypes.h>
+
+#include "../perf.h"
+#include "debug.h"
+#include "time-utils.h"
+#include "util.h"
+
+static int parse_timestr_sec_nsec(struct perf_time *ptime,
+				  char *start_str, char *end_str)
+{
+	if (start_str && (*start_str != '\0') &&
+	    (parse_nsec_time(start_str, &ptime->start) != 0)) {
+		return -1;
+	}
+
+	if (end_str && (*end_str != '\0') &&
+	    (parse_nsec_time(end_str, &ptime->end) != 0)) {
+		return -1;
+	}
+
+	return 0;
+}
+
+int perf_time__parse_str(struct perf_time *ptime, const char *ostr)
+{
+	char *start_str, *end_str;
+	char *d, *str;
+	int rc = 0;
+
+	if (ostr == NULL || *ostr == '\0')
+		return 0;
+
+	/* copy original string because we need to modify it */
+	str = strdup(ostr);
+	if (str == NULL)
+		return -ENOMEM;
+
+	ptime->start = 0;
+	ptime->end = 0;
+
+	/* str has the format: <start>,<stop>
+	 * variations: <start>,
+	 *             ,<stop>
+	 *             ,
+	 */
+	start_str = str;
+	d = strchr(start_str, ',');
+	if (d) {
+		*d = '\0';
+		++d;
+	}
+	end_str = d;
+
+	rc = parse_timestr_sec_nsec(ptime, start_str, end_str);
+
+	free(str);
+
+	/* make sure end time is after start time if it was given */
+	if (rc == 0 && ptime->end && ptime->end < ptime->start)
+		return -EINVAL;
+
+	pr_debug("start time %" PRIu64 ", ", ptime->start);
+	pr_debug("end time %" PRIu64 "\n", ptime->end);
+
+	return rc;
+}
+
+bool perf_time__skip_sample(struct perf_time *ptime, u64 timestamp)
+{
+	/* if time is not set don't drop sample */
+	if (timestamp == 0)
+		return false;
+
+	/* otherwise compare sample time to time window */
+	if ((ptime->start && timestamp < ptime->start) ||
+	    (ptime->end && timestamp > ptime->end)) {
+		return true;
+	}
+
+	return false;
+}
diff --git a/tools/perf/util/time-utils.h b/tools/perf/util/time-utils.h
new file mode 100644
index 000000000000..4368a481251d
--- /dev/null
+++ b/tools/perf/util/time-utils.h
@@ -0,0 +1,12 @@
+#ifndef _TIME_UTILS_H_
+#define _TIME_UTILS_H_
+
+struct perf_time {
+	u64 start, end;
+};
+
+int perf_time__parse_str(struct perf_time *ptime, const char *ostr);
+
+bool perf_time__skip_sample(struct perf_time *ptime, u64 timestamp);
+
+#endif
-- 
2.7.4 (Apple Git-66)

[toc] | [next] | [standalone]

#1531323

On Fri, Nov 25, 2016 at 02:39:54PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Add function to parse a user time string of the form <start>,<stop>
> where start and stop are time in sec.nsec format. Both start and stop
> times are optional.
> 
> Add function to determine if a sample time is within a given time
> time window of interest.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  tools/perf/util/Build        |  1 +
>  tools/perf/util/time-utils.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/time-utils.h | 12 +++++++
>  3 files changed, 98 insertions(+)
>  create mode 100644 tools/perf/util/time-utils.c
>  create mode 100644 tools/perf/util/time-utils.h
> 
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 1dc67efad634..78f139978e7a 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -87,6 +87,7 @@ libperf-y += help-unknown-cmd.o
>  libperf-y += mem-events.o
>  libperf-y += vsprintf.o
>  libperf-y += drv_configs.o
> +libperf-y += time-utils.o
>  
>  libperf-$(CONFIG_LIBBPF) += bpf-loader.o
>  libperf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o
> diff --git a/tools/perf/util/time-utils.c b/tools/perf/util/time-utils.c
> new file mode 100644
> index 000000000000..e584aeae9834
> --- /dev/null
> +++ b/tools/perf/util/time-utils.c
> @@ -0,0 +1,85 @@
> +#include <string.h>
> +#include <sys/time.h>
> +#include <time.h>
> +#include <errno.h>
> +#include <inttypes.h>
> +
> +#include "../perf.h"

could be just "perf.h"

jirka

[toc] | [prev] | [next] | [standalone]

#1531326

On Fri, Nov 25, 2016 at 02:39:54PM -0700, David Ahern wrote:

SNIP

> diff --git a/tools/perf/util/time-utils.h b/tools/perf/util/time-utils.h
> new file mode 100644
> index 000000000000..4368a481251d
> --- /dev/null
> +++ b/tools/perf/util/time-utils.h
> @@ -0,0 +1,12 @@
> +#ifndef _TIME_UTILS_H_
> +#define _TIME_UTILS_H_
> +
> +struct perf_time {
> +	u64 start, end;
> +};

hum, it's more interval rather than 'time'
would perf_interval, perf_time_interval suit better?

thanks,
jirka

[toc] | [prev] | [next] | [standalone]

#1531463

On 11/28/16 6:58 AM, Jiri Olsa wrote:
> On Fri, Nov 25, 2016 at 02:39:54PM -0700, David Ahern wrote:
> 
> SNIP
> 
>> diff --git a/tools/perf/util/time-utils.h b/tools/perf/util/time-utils.h
>> new file mode 100644
>> index 000000000000..4368a481251d
>> --- /dev/null
>> +++ b/tools/perf/util/time-utils.h
>> @@ -0,0 +1,12 @@
>> +#ifndef _TIME_UTILS_H_
>> +#define _TIME_UTILS_H_
>> +
>> +struct perf_time {
>> +	u64 start, end;
>> +};
> 
> hum, it's more interval rather than 'time'
> would perf_interval, perf_time_interval suit better?

I'll flip to perf_interval.

ack to the other 2 comments on this patch.

[toc] | [prev] | [next] | [standalone]

#1532455

On 11/29/16 9:02 AM, Arnaldo Carvalho de Melo wrote:
> Humm, I'd prefer 'time_interval' or 'perf_time_interval', plain
> 'interval' doesn't convey what kind if interval is this, we could be
> talking about counter values intervals, etc.

agreed, perf_time_interval makes more sense. 

[toc] | [prev] | [next] | [standalone]

#1532456

Em Mon, Nov 28, 2016 at 10:27:08AM -0700, David Ahern escreveu:
> On 11/28/16 6:58 AM, Jiri Olsa wrote:
> > On Fri, Nov 25, 2016 at 02:39:54PM -0700, David Ahern wrote:
> > 
> > SNIP
> > 
> >> diff --git a/tools/perf/util/time-utils.h b/tools/perf/util/time-utils.h
> >> new file mode 100644
> >> index 000000000000..4368a481251d
> >> --- /dev/null
> >> +++ b/tools/perf/util/time-utils.h
> >> @@ -0,0 +1,12 @@
> >> +#ifndef _TIME_UTILS_H_
> >> +#define _TIME_UTILS_H_
> >> +
> >> +struct perf_time {
> >> +	u64 start, end;
> >> +};
> > 
> > hum, it's more interval rather than 'time'
> > would perf_interval, perf_time_interval suit better?
> 
> I'll flip to perf_interval.

Humm, I'd prefer 'time_interval' or 'perf_time_interval', plain
'interval' doesn't convey what kind if interval is this, we could be
talking about counter values intervals, etc.

I was even expecting libc or POSIX to have something like this, but from
a quick look I couldn't find anything :-\
 
> ack to the other 2 comments on this patch.

Ok, waiting for v2 then.

- Arnaldo

[toc] | [prev] | [next] | [standalone]

#1531327

On Fri, Nov 25, 2016 at 02:39:54PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Add function to parse a user time string of the form <start>,<stop>
> where start and stop are time in sec.nsec format. Both start and stop
> times are optional.
> 
> Add function to determine if a sample time is within a given time
> time window of interest.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  tools/perf/util/Build        |  1 +
>  tools/perf/util/time-utils.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/time-utils.h | 12 +++++++
>  3 files changed, 98 insertions(+)
>  create mode 100644 tools/perf/util/time-utils.c
>  create mode 100644 tools/perf/util/time-utils.h
> 
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 1dc67efad634..78f139978e7a 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -87,6 +87,7 @@ libperf-y += help-unknown-cmd.o
>  libperf-y += mem-events.o
>  libperf-y += vsprintf.o
>  libperf-y += drv_configs.o
> +libperf-y += time-utils.o

I think we should call it just time.c, it's already in 'util' directory

jirka

[toc] | [prev] | [next] | [standalone]

#1531485

On 11/28/16 6:58 AM, Jiri Olsa wrote:
> On Fri, Nov 25, 2016 at 02:39:54PM -0700, David Ahern wrote:
>> From: David Ahern <dsahern@gmail.com>
>>
>> Add function to parse a user time string of the form <start>,<stop>
>> where start and stop are time in sec.nsec format. Both start and stop
>> times are optional.
>>
>> Add function to determine if a sample time is within a given time
>> time window of interest.
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> ---
>>  tools/perf/util/Build        |  1 +
>>  tools/perf/util/time-utils.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
>>  tools/perf/util/time-utils.h | 12 +++++++
>>  3 files changed, 98 insertions(+)
>>  create mode 100644 tools/perf/util/time-utils.c
>>  create mode 100644 tools/perf/util/time-utils.h
>>
>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
>> index 1dc67efad634..78f139978e7a 100644
>> --- a/tools/perf/util/Build
>> +++ b/tools/perf/util/Build
>> @@ -87,6 +87,7 @@ libperf-y += help-unknown-cmd.o
>>  libperf-y += mem-events.o
>>  libperf-y += vsprintf.o
>>  libperf-y += drv_configs.o
>> +libperf-y += time-utils.o
> 
> I think we should call it just time.c, it's already in 'util' directory

can't rename time-utils.h to time.h because the include will get confused with <time.h> and if the header is named time-utils.h why not keep the c-file as time-utils.c?

[toc] | [prev] | [next] | [standalone]

#1531498

On Mon, Nov 28, 2016 at 10:35:15AM -0700, David Ahern wrote:
> On 11/28/16 6:58 AM, Jiri Olsa wrote:
> > On Fri, Nov 25, 2016 at 02:39:54PM -0700, David Ahern wrote:
> >> From: David Ahern <dsahern@gmail.com>
> >>
> >> Add function to parse a user time string of the form <start>,<stop>
> >> where start and stop are time in sec.nsec format. Both start and stop
> >> times are optional.
> >>
> >> Add function to determine if a sample time is within a given time
> >> time window of interest.
> >>
> >> Signed-off-by: David Ahern <dsahern@gmail.com>
> >> ---
> >>  tools/perf/util/Build        |  1 +
> >>  tools/perf/util/time-utils.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
> >>  tools/perf/util/time-utils.h | 12 +++++++
> >>  3 files changed, 98 insertions(+)
> >>  create mode 100644 tools/perf/util/time-utils.c
> >>  create mode 100644 tools/perf/util/time-utils.h
> >>
> >> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> >> index 1dc67efad634..78f139978e7a 100644
> >> --- a/tools/perf/util/Build
> >> +++ b/tools/perf/util/Build
> >> @@ -87,6 +87,7 @@ libperf-y += help-unknown-cmd.o
> >>  libperf-y += mem-events.o
> >>  libperf-y += vsprintf.o
> >>  libperf-y += drv_configs.o
> >> +libperf-y += time-utils.o
> > 
> > I think we should call it just time.c, it's already in 'util' directory
> 
> can't rename time-utils.h to time.h because the include will get confused with <time.h> and if the header is named time-utils.h why not keep the c-file as time-utils.c?
> 

ok, haven't realized that.. can't think of another name ;-)

there are some time related functions in util.[ch], maybe you
coudl move them as well

int fetch_current_timestamp(char *buf, size_t sz);
int timestamp__scnprintf_usec(u64 timestamp, char *buf, size_t sz);

jirka

[toc] | [prev] | [next] | [standalone]

#1531502

On 11/28/16 11:14 AM, Jiri Olsa wrote:
> there are some time related functions in util.[ch], maybe you
> coudl move them as well
> 
> int fetch_current_timestamp(char *buf, size_t sz);
> int timestamp__scnprintf_usec(u64 timestamp, char *buf, size_t sz);

I'd like to defer that to a follow on patch.

I only move parse_nsec_time in this set because there are no other users but the one introduced in patch 1.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web