All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] trace-cmd: Add record --clock option to specify a trace clock
@ 2013-12-11 19:08 Ian Webster
  2013-12-11 19:26 ` Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Ian Webster @ 2013-12-11 19:08 UTC (permalink / raw)
  To: rostedt; +Cc: linux-kernel

This change adds a --clock option to trace-cmd record. It simply writes
trace_clock on debugfs. Examples of valid choices on most systems are: local,
global, counter, and any other choice compatible with ftrace.

This complements Yoshihiro Yunomae's change 8c9867c (trace-cmd: Add support for
extracting trace_clock in report).

Signed-off-by: Ian Webster <ian@planetaryresources.com>
---
Documentation/trace-cmd-record.1.txt |    5 +++++
trace-record.c                       |   19 +++++++++++++++++++
trace-usage.c                        |    1 +
3 files changed, 25 insertions(+)

diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt
index 832a257..201be2b 100644
--- a/Documentation/trace-cmd-record.1.txt
+++ b/Documentation/trace-cmd-record.1.txt
@@ -240,6 +240,11 @@ OPTIONS
     timestamp to gettimeofday which will allow wall time output from the
     timestamps reading the created 'trace.dat' file.

+*--clock* 'clock_name'::
+    When an event is recorded into the ring buffer, a timestamp is added. This
+    stamp comes from the specified clock. Ftrace uses the "local" clock by
+    default. Other common clocks include: global, counter, uptime, perf, x86-tsc.
+
EXAMPLES
--------

diff --git a/trace-record.c b/trace-record.c
index 0199627..18de026 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -399,6 +399,20 @@ static int set_ftrace(int set, int use_proc)
  return 0;
}

+static void set_trace_clock(char *clock_choice)
+{
+ FILE *fp;
+ char *path;
+
+ path = tracecmd_get_tracing_file("trace_clock");
+ fp = fopen(path, "w");
+ if (!fp)
+   die("writing to '%s'", path);
+ tracecmd_put_tracing_file(path);
+ fwrite(clock_choice, 1, strlen(clock_choice), fp);
+ fclose(fp);
+}
+
static char *
get_instance_file(struct buffer_instance *instance, const char *file)
{
@@ -2279,6 +2293,7 @@ static void record_all_events(void)
}

enum {
+ OPT_clock = 252,
  OPT_nosplice  = 253,
  OPT_funcstack = 254,
  OPT_date  = 255,
@@ -2351,6 +2366,7 @@ void trace_record (int argc, char **argv)
      {"func-stack", no_argument, NULL, OPT_funcstack},
      {"nosplice", no_argument, NULL, OPT_nosplice},
      {"help", no_argument, NULL, '?'},
+     {"clock", required_argument, NULL, OPT_clock},
      {NULL, 0, NULL, 0}
    };

@@ -2530,6 +2546,9 @@ void trace_record (int argc, char **argv)
    case OPT_nosplice:
      recorder_flags |= TRACECMD_RECORD_NOSPLICE;
      break;
+   case OPT_clock:
+     set_trace_clock(optarg);
+     break;
    default:
      usage(argv);
    }
diff --git a/trace-usage.c b/trace-usage.c
index b8f26e6..54dbb85 100644
--- a/trace-usage.c
+++ b/trace-usage.c
@@ -46,6 +46,7 @@ static struct usage_help usage_help[] = {
    "          -i do not fail if an event is not found\n"
    "          --func-stack perform a stack trace for function tracer\n"
    "             (use with caution)\n"
+   "          --clock change the method used to produce trace timestamps\n"
  },
  {
    "start",
--
1.7.9.5



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] trace-cmd: Add record --clock option to specify a trace clock
  2013-12-11 19:08 [PATCH] trace-cmd: Add record --clock option to specify a trace clock Ian Webster
@ 2013-12-11 19:26 ` Steven Rostedt
  2013-12-12 23:30   ` [PATCH v2] " Ian Webster
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2013-12-11 19:26 UTC (permalink / raw)
  To: Ian Webster; +Cc: linux-kernel

On Wed, 11 Dec 2013 11:08:32 -0800
Ian Webster <ian@planetaryresources.com> wrote:

> This change adds a --clock option to trace-cmd record. It simply writes
> trace_clock on debugfs. Examples of valid choices on most systems are: local,
> global, counter, and any other choice compatible with ftrace.
> 
> This complements Yoshihiro Yunomae's change 8c9867c (trace-cmd: Add support for
> extracting trace_clock in report).
> 
> Signed-off-by: Ian Webster <ian@planetaryresources.com>
> ---
> Documentation/trace-cmd-record.1.txt |    5 +++++
> trace-record.c                       |   19 +++++++++++++++++++
> trace-usage.c                        |    1 +
> 3 files changed, 25 insertions(+)
> 
> diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt
> index 832a257..201be2b 100644
> --- a/Documentation/trace-cmd-record.1.txt
> +++ b/Documentation/trace-cmd-record.1.txt
> @@ -240,6 +240,11 @@ OPTIONS
>      timestamp to gettimeofday which will allow wall time output from the
>      timestamps reading the created 'trace.dat' file.
> 
> +*--clock* 'clock_name'::
> +    When an event is recorded into the ring buffer, a timestamp is added. This
> +    stamp comes from the specified clock. Ftrace uses the "local" clock by
> +    default. Other common clocks include: global, counter, uptime, perf, x86-tsc.
> +
> EXAMPLES
> --------
> 
> diff --git a/trace-record.c b/trace-record.c
> index 0199627..18de026 100644
> --- a/trace-record.c
> +++ b/trace-record.c
> @@ -399,6 +399,20 @@ static int set_ftrace(int set, int use_proc)
>   return 0;
> }
> 
> +static void set_trace_clock(char *clock_choice)

Should be const char *clock_choice.

> +{
> + FILE *fp;
> + char *path;
> +
> + path = tracecmd_get_tracing_file("trace_clock");
> + fp = fopen(path, "w");
> + if (!fp)
> +   die("writing to '%s'", path);
> + tracecmd_put_tracing_file(path);
> + fwrite(clock_choice, 1, strlen(clock_choice), fp);
> + fclose(fp);

Whitespace damage here.

I like the change, but the patch needs to be fixed. The tabs turned to
spaces and the clock_choice above needs to be const.

Thanks,

-- Steve


> +}
> +
> static char *
> get_instance_file(struct buffer_instance *instance, const char *file)
> {
> @@ -2279,6 +2293,7 @@ static void record_all_events(void)
> }
> 
> enum {
> + OPT_clock = 252,
>   OPT_nosplice  = 253,
>   OPT_funcstack = 254,
>   OPT_date  = 255,
> @@ -2351,6 +2366,7 @@ void trace_record (int argc, char **argv)
>       {"func-stack", no_argument, NULL, OPT_funcstack},
>       {"nosplice", no_argument, NULL, OPT_nosplice},
>       {"help", no_argument, NULL, '?'},
> +     {"clock", required_argument, NULL, OPT_clock},
>       {NULL, 0, NULL, 0}
>     };
> 
> @@ -2530,6 +2546,9 @@ void trace_record (int argc, char **argv)
>     case OPT_nosplice:
>       recorder_flags |= TRACECMD_RECORD_NOSPLICE;
>       break;
> +   case OPT_clock:
> +     set_trace_clock(optarg);
> +     break;
>     default:
>       usage(argv);
>     }
> diff --git a/trace-usage.c b/trace-usage.c
> index b8f26e6..54dbb85 100644
> --- a/trace-usage.c
> +++ b/trace-usage.c
> @@ -46,6 +46,7 @@ static struct usage_help usage_help[] = {
>     "          -i do not fail if an event is not found\n"
>     "          --func-stack perform a stack trace for function tracer\n"
>     "             (use with caution)\n"
> +   "          --clock change the method used to produce trace timestamps\n"
>   },
>   {
>     "start",
> --
> 1.7.9.5
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2] trace-cmd: Add record --clock option to specify a trace clock
  2013-12-11 19:26 ` Steven Rostedt
@ 2013-12-12 23:30   ` Ian Webster
  2013-12-16  4:10     ` Yoshihiro YUNOMAE
  0 siblings, 1 reply; 5+ messages in thread
From: Ian Webster @ 2013-12-12 23:30 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, yoshihiro.yunomae.ez

This change adds a --clock option to trace-cmd record. It simply writes
trace_clock on debugfs. Examples of valid choices on most systems are: local,
global, counter, and any other choice compatible with ftrace.

This complements Yoshihiro Yunomae's change 8c9867c (trace-cmd: Add support for
extracting trace_clock in report).

Signed-off-by: Ian Webster <ian@planetaryresources.com>

---
Changes per Steven's comments:
  * add missing const
  * preserve tab whitespace

 Documentation/trace-cmd-record.1.txt |    5 +++++
 trace-record.c                       |   19 +++++++++++++++++++
 trace-usage.c                        |    1 +
 3 files changed, 25 insertions(+)

diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt
index 832a257..5e83454 100644
--- a/Documentation/trace-cmd-record.1.txt
+++ b/Documentation/trace-cmd-record.1.txt
@@ -240,6 +240,11 @@ OPTIONS
     timestamp to gettimeofday which will allow wall time output from the
     timestamps reading the created 'trace.dat' file.

+*--clock* 'clock_name'::
+    When an event is recorded into the ring buffer, a timestamp is added. This
+    stamp comes from the specified clock. Ftrace uses the "local" clock by
+    default. Other common clocks include: global, counter, uptime, perf, x86-tsc.
+
 EXAMPLES
 --------

diff --git a/trace-record.c b/trace-record.c
index 0199627..2143080 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -399,6 +399,20 @@ static int set_ftrace(int set, int use_proc)
 	return 0;
 }

+static void set_trace_clock(const char *clock_choice)
+{
+	FILE *fp;
+	char *path;
+
+	path = tracecmd_get_tracing_file("trace_clock");
+	fp = fopen(path, "w");
+	if (!fp)
+		die("writing to '%s'", path);
+	tracecmd_put_tracing_file(path);
+	fwrite(clock_choice, 1, strlen(clock_choice), fp);
+	fclose(fp);
+}
+
 static char *
 get_instance_file(struct buffer_instance *instance, const char *file)
 {
@@ -2279,6 +2293,7 @@ static void record_all_events(void)
 }

 enum {
+	OPT_clock = 252,
 	OPT_nosplice	= 253,
 	OPT_funcstack	= 254,
 	OPT_date	= 255,
@@ -2351,6 +2366,7 @@ void trace_record (int argc, char **argv)
 			{"func-stack", no_argument, NULL, OPT_funcstack},
 			{"nosplice", no_argument, NULL, OPT_nosplice},
 			{"help", no_argument, NULL, '?'},
+			{"clock", required_argument, NULL, OPT_clock},
 			{NULL, 0, NULL, 0}
 		};

@@ -2530,6 +2546,9 @@ void trace_record (int argc, char **argv)
 		case OPT_nosplice:
 			recorder_flags |= TRACECMD_RECORD_NOSPLICE;
 			break;
+		case OPT_clock:
+			set_trace_clock(optarg);
+			break;
 		default:
 			usage(argv);
 		}
diff --git a/trace-usage.c b/trace-usage.c
index b8f26e6..54dbb85 100644
--- a/trace-usage.c
+++ b/trace-usage.c
@@ -46,6 +46,7 @@ static struct usage_help usage_help[] = {
 		"          -i do not fail if an event is not found\n"
 		"          --func-stack perform a stack trace for function tracer\n"
 		"             (use with caution)\n"
+		"          --clock change the method used to produce trace timestamps\n"
 	},
 	{
 		"start",
--
1.7.9.5


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] trace-cmd: Add record --clock option to specify a trace clock
  2013-12-12 23:30   ` [PATCH v2] " Ian Webster
@ 2013-12-16  4:10     ` Yoshihiro YUNOMAE
  2015-05-14 14:32       ` Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Yoshihiro YUNOMAE @ 2013-12-16  4:10 UTC (permalink / raw)
  To: Ian Webster; +Cc: Steven Rostedt, linux-kernel

Hi Ian,

(2013/12/13 8:30), Ian Webster wrote:
> This change adds a --clock option to trace-cmd record. It simply writes
> trace_clock on debugfs. Examples of valid choices on most systems are: local,
> global, counter, and any other choice compatible with ftrace.

Nice work!
Please see my following comments.

> This complements Yoshihiro Yunomae's change 8c9867c (trace-cmd: Add support for
> extracting trace_clock in report).
>
> Signed-off-by: Ian Webster <ian@planetaryresources.com>
>
> ---
> Changes per Steven's comments:
>    * add missing const
>    * preserve tab whitespace
>
>   Documentation/trace-cmd-record.1.txt |    5 +++++
>   trace-record.c                       |   19 +++++++++++++++++++
>   trace-usage.c                        |    1 +
>   3 files changed, 25 insertions(+)
>
> diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt
> index 832a257..5e83454 100644
> --- a/Documentation/trace-cmd-record.1.txt
> +++ b/Documentation/trace-cmd-record.1.txt
> @@ -240,6 +240,11 @@ OPTIONS
>       timestamp to gettimeofday which will allow wall time output from the
>       timestamps reading the created 'trace.dat' file.
>
> +*--clock* 'clock_name'::
> +    When an event is recorded into the ring buffer, a timestamp is added. This
> +    stamp comes from the specified clock. Ftrace uses the "local" clock by
> +    default. Other common clocks include: global, counter, uptime, perf, x86-tsc.
> +
>   EXAMPLES
>   --------
>
> diff --git a/trace-record.c b/trace-record.c
> index 0199627..2143080 100644
> --- a/trace-record.c
> +++ b/trace-record.c
> @@ -399,6 +399,20 @@ static int set_ftrace(int set, int use_proc)
>   	return 0;
>   }
>
> +static void set_trace_clock(const char *clock_choice)
> +{
> +	FILE *fp;
> +	char *path;
> +
> +	path = tracecmd_get_tracing_file("trace_clock");
> +	fp = fopen(path, "w");
> +	if (!fp)
> +		die("writing to '%s'", path);
> +	tracecmd_put_tracing_file(path);
> +	fwrite(clock_choice, 1, strlen(clock_choice), fp);

Please add an error check for fwrite(). If clock_choice is invalid,
trace-cmd would better output the error report and die.

> +	fclose(fp);
> +}
> +
>   static char *
>   get_instance_file(struct buffer_instance *instance, const char *file)
>   {
> @@ -2279,6 +2293,7 @@ static void record_all_events(void)
>   }
>
>   enum {
> +	OPT_clock = 252,
>   	OPT_nosplice	= 253,
>   	OPT_funcstack	= 254,
>   	OPT_date	= 255,
> @@ -2351,6 +2366,7 @@ void trace_record (int argc, char **argv)
>   			{"func-stack", no_argument, NULL, OPT_funcstack},
>   			{"nosplice", no_argument, NULL, OPT_nosplice},
>   			{"help", no_argument, NULL, '?'},
> +			{"clock", required_argument, NULL, OPT_clock},
>   			{NULL, 0, NULL, 0}
>   		};
>
> @@ -2530,6 +2546,9 @@ void trace_record (int argc, char **argv)
>   		case OPT_nosplice:
>   			recorder_flags |= TRACECMD_RECORD_NOSPLICE;
>   			break;
> +		case OPT_clock:
> +			set_trace_clock(optarg);
> +			break;

This implementation does not support multiple buffers.
trace_clock had better be changed after sub-buffers are created, I
think.

Example:
trace-cmd record -e sched --clock local -B foo -e block --clock x86-tsc

Thanks,
Yoshihiro YUNOMAE

-- 
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@hitachi.com



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] trace-cmd: Add record --clock option to specify a trace clock
  2013-12-16  4:10     ` Yoshihiro YUNOMAE
@ 2015-05-14 14:32       ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2015-05-14 14:32 UTC (permalink / raw)
  To: Yoshihiro YUNOMAE; +Cc: Ian Webster, linux-kernel

On Mon, 16 Dec 2013 13:10:52 +0900
Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@hitachi.com> wrote:

> Hi Ian,
> 
> (2013/12/13 8:30), Ian Webster wrote:
> > This change adds a --clock option to trace-cmd record. It simply writes
> > trace_clock on debugfs. Examples of valid choices on most systems are: local,
> > global, counter, and any other choice compatible with ftrace.
> 
> Nice work!
> Please see my following comments.

Hmm, I was waiting for a reply back with an update against these
comments and never saw one. I eventually forgot about this patch :-(

I went to add it now, and noticed that I already added a -C option for
trace-clock which also works for instances.

Sorry about forgetting about this patch (just found it buried in my
inbox).

I'll be releasing 2.6 soon, with a bunch of new updates.

-- Steve

> 
> > This complements Yoshihiro Yunomae's change 8c9867c (trace-cmd: Add support for
> > extracting trace_clock in report).
> >
> > Signed-off-by: Ian Webster <ian@planetaryresources.com>
> >
> > ---
> > Changes per Steven's comments:
> >    * add missing const
> >    * preserve tab whitespace
> >
> >   Documentation/trace-cmd-record.1.txt |    5 +++++
> >   trace-record.c                       |   19 +++++++++++++++++++
> >   trace-usage.c                        |    1 +
> >   3 files changed, 25 insertions(+)
> >
> > diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt
> > index 832a257..5e83454 100644
> > --- a/Documentation/trace-cmd-record.1.txt
> > +++ b/Documentation/trace-cmd-record.1.txt
> > @@ -240,6 +240,11 @@ OPTIONS
> >       timestamp to gettimeofday which will allow wall time output from the
> >       timestamps reading the created 'trace.dat' file.
> >
> > +*--clock* 'clock_name'::
> > +    When an event is recorded into the ring buffer, a timestamp is added. This
> > +    stamp comes from the specified clock. Ftrace uses the "local" clock by
> > +    default. Other common clocks include: global, counter, uptime, perf, x86-tsc.
> > +
> >   EXAMPLES
> >   --------
> >
> > diff --git a/trace-record.c b/trace-record.c
> > index 0199627..2143080 100644
> > --- a/trace-record.c
> > +++ b/trace-record.c
> > @@ -399,6 +399,20 @@ static int set_ftrace(int set, int use_proc)
> >   	return 0;
> >   }
> >
> > +static void set_trace_clock(const char *clock_choice)
> > +{
> > +	FILE *fp;
> > +	char *path;
> > +
> > +	path = tracecmd_get_tracing_file("trace_clock");
> > +	fp = fopen(path, "w");
> > +	if (!fp)
> > +		die("writing to '%s'", path);
> > +	tracecmd_put_tracing_file(path);
> > +	fwrite(clock_choice, 1, strlen(clock_choice), fp);
> 
> Please add an error check for fwrite(). If clock_choice is invalid,
> trace-cmd would better output the error report and die.
> 
> > +	fclose(fp);
> > +}
> > +
> >   static char *
> >   get_instance_file(struct buffer_instance *instance, const char *file)
> >   {
> > @@ -2279,6 +2293,7 @@ static void record_all_events(void)
> >   }
> >
> >   enum {
> > +	OPT_clock = 252,
> >   	OPT_nosplice	= 253,
> >   	OPT_funcstack	= 254,
> >   	OPT_date	= 255,
> > @@ -2351,6 +2366,7 @@ void trace_record (int argc, char **argv)
> >   			{"func-stack", no_argument, NULL, OPT_funcstack},
> >   			{"nosplice", no_argument, NULL, OPT_nosplice},
> >   			{"help", no_argument, NULL, '?'},
> > +			{"clock", required_argument, NULL, OPT_clock},
> >   			{NULL, 0, NULL, 0}
> >   		};
> >
> > @@ -2530,6 +2546,9 @@ void trace_record (int argc, char **argv)
> >   		case OPT_nosplice:
> >   			recorder_flags |= TRACECMD_RECORD_NOSPLICE;
> >   			break;
> > +		case OPT_clock:
> > +			set_trace_clock(optarg);
> > +			break;
> 
> This implementation does not support multiple buffers.
> trace_clock had better be changed after sub-buffers are created, I
> think.
> 
> Example:
> trace-cmd record -e sched --clock local -B foo -e block --clock x86-tsc
> 
> Thanks,
> Yoshihiro YUNOMAE
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-05-14 14:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-11 19:08 [PATCH] trace-cmd: Add record --clock option to specify a trace clock Ian Webster
2013-12-11 19:26 ` Steven Rostedt
2013-12-12 23:30   ` [PATCH v2] " Ian Webster
2013-12-16  4:10     ` Yoshihiro YUNOMAE
2015-05-14 14:32       ` Steven Rostedt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.