linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] New trace-cmd "set" command and changes in "start"
@ 2020-06-03 12:15 Tzvetomir Stoyanov (VMware)
  2020-06-03 12:15 ` [PATCH v3 1/5] trace-cmd: Enable "trace-cmd start" to run a command Tzvetomir Stoyanov (VMware)
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-06-03 12:15 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

"trace-cmd start": Enable to run and filter a command.
"trace-cmd set": Introduce a new sub command. It works the same way as
"start", except it does not clear the ftrace state.

v3 changes:
  - Added new path in the set:
   "trace-cmd: Extend option "-v" to delete an ftrace instance" 

Tzvetomir Stoyanov (VMware) (5):
  trace-cmd: Enable "trace-cmd start" to run a command
  trace-cmd: Add new subcommand "set"
  trace-cmd: Man page for "set" subcommand
  trace-cmd: Extend option "-v" to delete an ftrace instance
  trace-cmd: Add description of "-G" option

 Documentation/trace-cmd-record.1.txt |   4 +
 Documentation/trace-cmd-set.1.txt    | 259 +++++++++++++++++++++++++++
 Documentation/trace-cmd-start.1.txt  |   2 +-
 tracecmd/include/trace-local.h       |   3 +
 tracecmd/trace-cmd.c                 |   1 +
 tracecmd/trace-record.c              | 134 ++++++++++++--
 tracecmd/trace-usage.c               |  40 ++++-
 7 files changed, 422 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/trace-cmd-set.1.txt

-- 
2.26.2


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

* [PATCH v3 1/5] trace-cmd: Enable "trace-cmd start" to run a command
  2020-06-03 12:15 [PATCH v3 0/5] New trace-cmd "set" command and changes in "start" Tzvetomir Stoyanov (VMware)
@ 2020-06-03 12:15 ` Tzvetomir Stoyanov (VMware)
  2020-06-08 19:35   ` Steven Rostedt
  2020-06-03 12:15 ` [PATCH v3 2/5] trace-cmd: Add new subcommand "set" Tzvetomir Stoyanov (VMware)
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-06-03 12:15 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

There is a resrtiction of "trace-cmd start" which does not allow it to
run a command, specified by "-F" option or at the end of the command
line. However, there are use cases where this is useful.
The resrtiction is removed, it allows "trace-cmd start" to run commands
just like "trace-cmd record".

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 Documentation/trace-cmd-start.1.txt |  2 +-
 tracecmd/trace-record.c             | 28 ++++++++++++++++++++--------
 tracecmd/trace-usage.c              |  2 +-
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/Documentation/trace-cmd-start.1.txt b/Documentation/trace-cmd-start.1.txt
index 2b027b01..63d2034c 100644
--- a/Documentation/trace-cmd-start.1.txt
+++ b/Documentation/trace-cmd-start.1.txt
@@ -21,7 +21,7 @@ system or can be extracted with trace-cmd-extract(1).
 OPTIONS
 -------
 The options are the same as 'trace-cmd-record(1)', except that it does not
-take options specific to recording (*-s*, *-o*, *-F*, *-N*, and *-t*).
+take options specific to recording (*-s*, *-o*, *-N*, and *-t*).
 
 SEE ALSO
 --------
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 3242368e..f345c6d6 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -1598,7 +1598,8 @@ static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
 		/* child */
 		update_task_filter();
 		tracecmd_enable_tracing();
-		enable_ptrace();
+		if (type != TRACE_TYPE_START)
+			enable_ptrace();
 		/*
 		 * If we are using stderr for stdout, switch
 		 * it back to the saved stdout for the code we run.
@@ -1619,6 +1620,8 @@ static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
 			die("Failed to exec %s", argv[0]);
 		}
 	}
+	if (type & TRACE_TYPE_START)
+		exit(0);
 	if (do_ptrace) {
 		ptrace_attach(NULL, pid);
 		ptrace_wait(type);
@@ -5782,6 +5785,14 @@ static void add_arg(struct buffer_instance *instance,
 	/* Not found? */
 }
 
+static inline void cmd_check_die(struct common_record_context *ctx,
+				 enum trace_cmd id, char *cmd, char *param)
+{
+	if (ctx->curr_cmd == id)
+		die("%s has no effect with the command %s\n"
+		    "Did you mean 'record'?", param, cmd);
+}
+
 static void parse_record_options(int argc,
 				 char **argv,
 				 enum trace_cmd curr_cmd,
@@ -6114,6 +6125,7 @@ static void parse_record_options(int argc,
 				die("Failed to allocate user name");
 			break;
 		case OPT_procmap:
+			cmd_check_die(ctx, CMD_start, *(argv+1), "--proc-map");
 			ctx->instance->get_procmap = 1;
 			break;
 		case OPT_date:
@@ -6221,9 +6233,6 @@ static void parse_record_options(int argc,
 		die(" -c can only be used with -F (or -P with event-fork support)");
 
 	if ((argc - optind) >= 2) {
-		if (IS_START(ctx))
-			die("Command start does not take any commands\n"
-			    "Did you mean 'record'?");
 		if (IS_EXTRACT(ctx))
 			die("Command extract does not take any commands\n"
 			    "Did you mean 'record'?");
@@ -6244,6 +6253,9 @@ static void parse_record_options(int argc,
 		}
 	}
 
+	if (do_ptrace && IS_START(ctx))
+		die("ptrace not supported with command start");
+
 	for_all_instances(instance) {
 		if (instance->get_procmap && !instance->nr_filter_pids) {
 			warning("--proc-map is ignored for instance %s, "
@@ -6410,10 +6422,6 @@ static void record_trace(int argc, char **argv,
 		signal(SIGINT, finish);
 		if (!latency)
 			start_threads(type, ctx);
-	} else {
-		update_task_filter();
-		tracecmd_enable_tracing();
-		exit(0);
 	}
 
 	if (ctx->run_command) {
@@ -6428,6 +6436,10 @@ static void record_trace(int argc, char **argv,
 
 		update_task_filter();
 		tracecmd_enable_tracing();
+
+		if (type & TRACE_TYPE_START)
+			exit(0);
+
 		/* We don't ptrace ourself */
 		if (do_ptrace) {
 			for_all_instances(instance) {
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index d43ca490..58bca9e4 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -69,7 +69,7 @@ static struct usage_help usage_help[] = {
 		"start",
 		"start tracing without recording into a file",
 		" %s start [-e event][-p plugin][-d][-O option ][-P pid]\n"
-		"          Uses same options as record, but does not run a command.\n"
+		"          Uses same options as record.\n"
 		"          It only enables the tracing and exits\n"
 	},
 	{
-- 
2.26.2


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

* [PATCH v3 2/5] trace-cmd: Add new subcommand "set"
  2020-06-03 12:15 [PATCH v3 0/5] New trace-cmd "set" command and changes in "start" Tzvetomir Stoyanov (VMware)
  2020-06-03 12:15 ` [PATCH v3 1/5] trace-cmd: Enable "trace-cmd start" to run a command Tzvetomir Stoyanov (VMware)
@ 2020-06-03 12:15 ` Tzvetomir Stoyanov (VMware)
  2020-06-03 12:15 ` [PATCH v3 3/5] trace-cmd: Man page for "set" subcommand Tzvetomir Stoyanov (VMware)
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-06-03 12:15 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

The command "set" configures various ftarce parameters, like "record"
and "start" commands, but does not reset ftrace state on exit.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=207781
Reported-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 tracecmd/include/trace-local.h |  2 ++
 tracecmd/trace-cmd.c           |  1 +
 tracecmd/trace-record.c        | 61 ++++++++++++++++++++++++++++------
 tracecmd/trace-usage.c         | 35 +++++++++++++++++++
 4 files changed, 89 insertions(+), 10 deletions(-)

diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 5d585509..5ec05149 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -57,6 +57,8 @@ void trace_reset(int argc, char **argv);
 
 void trace_start(int argc, char **argv);
 
+void trace_set(int argc, char **argv);
+
 void trace_extract(int argc, char **argv);
 
 void trace_stream(int argc, char **argv);
diff --git a/tracecmd/trace-cmd.c b/tracecmd/trace-cmd.c
index dbfcc974..7376c5a5 100644
--- a/tracecmd/trace-cmd.c
+++ b/tracecmd/trace-cmd.c
@@ -90,6 +90,7 @@ struct command commands[] = {
 	{"check-events", trace_check_events},
 	{"record", trace_record},
 	{"start", trace_start},
+	{"set", trace_set},
 	{"extract", trace_extract},
 	{"stop", trace_stop},
 	{"stream", trace_stream},
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index f345c6d6..d5515c52 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -63,6 +63,7 @@ enum trace_type {
 	TRACE_TYPE_START	= (1 << 1),
 	TRACE_TYPE_STREAM	= (1 << 2),
 	TRACE_TYPE_EXTRACT	= (1 << 3),
+	TRACE_TYPE_SET		= (1 << 4),
 };
 
 static tracecmd_handle_init_func handle_init = NULL;
@@ -185,6 +186,7 @@ enum trace_cmd {
 	CMD_profile,
 	CMD_record,
 	CMD_record_agent,
+	CMD_set,
 };
 
 struct common_record_context {
@@ -1598,7 +1600,8 @@ static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
 		/* child */
 		update_task_filter();
 		tracecmd_enable_tracing();
-		if (type != TRACE_TYPE_START)
+		if (type != TRACE_TYPE_START &&
+		    type != TRACE_TYPE_SET)
 			enable_ptrace();
 		/*
 		 * If we are using stderr for stdout, switch
@@ -1620,7 +1623,7 @@ static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
 			die("Failed to exec %s", argv[0]);
 		}
 	}
-	if (type & TRACE_TYPE_START)
+	if (type & (TRACE_TYPE_START | TRACE_TYPE_SET))
 		exit(0);
 	if (do_ptrace) {
 		ptrace_attach(NULL, pid);
@@ -5722,6 +5725,7 @@ static void init_common_record_context(struct common_record_context *ctx,
 
 #define IS_EXTRACT(ctx) ((ctx)->curr_cmd == CMD_extract)
 #define IS_START(ctx) ((ctx)->curr_cmd == CMD_start)
+#define IS_CMDSET(ctx) ((ctx)->curr_cmd == CMD_set)
 #define IS_STREAM(ctx) ((ctx)->curr_cmd == CMD_stream)
 #define IS_PROFILE(ctx) ((ctx)->curr_cmd == CMD_profile)
 #define IS_RECORD(ctx) ((ctx)->curr_cmd == CMD_record)
@@ -5814,6 +5818,9 @@ static void parse_record_options(int argc,
 
 	init_common_record_context(ctx, curr_cmd);
 
+	if (IS_CMDSET(ctx))
+		keep = 1;
+
 	for (;;) {
 		int option_index = 0;
 		int ret;
@@ -5865,6 +5872,7 @@ static void parse_record_options(int argc,
 			usage(argv);
 			break;
 		case 'a':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-a");
 			if (IS_EXTRACT(ctx)) {
 				add_all_instances();
 			} else {
@@ -5942,6 +5950,7 @@ static void parse_record_options(int argc,
 			filter_task = 1;
 			break;
 		case 'G':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-G");
 			ctx->global = 1;
 			break;
 		case 'P':
@@ -6017,6 +6026,7 @@ static void parse_record_options(int argc,
 			ctx->disable = 1;
 			break;
 		case 'o':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-o");
 			if (IS_RECORD_AGENT(ctx))
 				die("-o incompatible with agent recording");
 			if (host)
@@ -6054,10 +6064,12 @@ static void parse_record_options(int argc,
 			save_option(ctx->instance, "stacktrace");
 			break;
 		case 'H':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-H");
 			add_hook(ctx->instance, optarg);
 			ctx->events = 1;
 			break;
 		case 's':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-s");
 			if (IS_EXTRACT(ctx)) {
 				if (optarg)
 					usage(argv);
@@ -6069,15 +6081,18 @@ static void parse_record_options(int argc,
 			sleep_time = atoi(optarg);
 			break;
 		case 'S':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-S");
 			ctx->manual = 1;
 			/* User sets events for profiling */
 			if (!event)
 				ctx->events = 0;
 			break;
 		case 'r':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-r");
 			rt_prio = atoi(optarg);
 			break;
 		case 'N':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-N");
 			if (!IS_RECORD(ctx))
 				die("-N only available with record");
 			if (IS_RECORD_AGENT(ctx))
@@ -6097,6 +6112,7 @@ static void parse_record_options(int argc,
 			ctx->instance->cpumask = alloc_mask_from_hex(ctx->instance, optarg);
 			break;
 		case 't':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-t");
 			if (IS_EXTRACT(ctx))
 				ctx->topt = 1; /* Extract top instance also */
 			else
@@ -6114,6 +6130,7 @@ static void parse_record_options(int argc,
 				ctx->instance->flags |= BUFFER_FL_PROFILE;
 			break;
 		case 'k':
+			cmd_check_die(ctx, CMD_set, *(argv+1), "-k");
 			keep = 1;
 			break;
 		case 'i':
@@ -6126,9 +6143,11 @@ static void parse_record_options(int argc,
 			break;
 		case OPT_procmap:
 			cmd_check_die(ctx, CMD_start, *(argv+1), "--proc-map");
+			cmd_check_die(ctx, CMD_set, *(argv+1), "--proc-map");
 			ctx->instance->get_procmap = 1;
 			break;
 		case OPT_date:
+			cmd_check_die(ctx, CMD_set, *(argv+1), "--date");
 			ctx->date = 1;
 			if (ctx->data_flags & DATA_FL_OFFSET)
 				die("Can not use both --date and --ts-offset");
@@ -6138,12 +6157,15 @@ static void parse_record_options(int argc,
 			func_stack = 1;
 			break;
 		case OPT_nosplice:
+			cmd_check_die(ctx, CMD_set, *(argv+1), "--nosplice");
 			recorder_flags |= TRACECMD_RECORD_NOSPLICE;
 			break;
 		case OPT_nofifos:
+			cmd_check_die(ctx, CMD_set, *(argv+1), "--nofifos");
 			no_fifos = true;
 			break;
 		case OPT_profile:
+			cmd_check_die(ctx, CMD_set, *(argv+1), "--profile");
 			handle_init = trace_init_profile;
 			ctx->instance->flags |= BUFFER_FL_PROFILE;
 			ctx->events = 1;
@@ -6157,9 +6179,11 @@ static void parse_record_options(int argc,
 			dup2(2, 1);
 			break;
 		case OPT_bycomm:
+			cmd_check_die(ctx, CMD_set, *(argv+1), "--by-comm");
 			trace_profile_set_merge_like_comms();
 			break;
 		case OPT_tsoffset:
+			cmd_check_die(ctx, CMD_set, *(argv+1), "--ts-offset");
 			ctx->date2ts = strdup(optarg);
 			if (ctx->data_flags & DATA_FL_DATE)
 				die("Can not use both --date and --ts-offset");
@@ -6175,6 +6199,7 @@ static void parse_record_options(int argc,
 			ctx->saved_cmdlines_size = atoi(optarg);
 			break;
 		case OPT_no_filter:
+			cmd_check_die(ctx, CMD_set, *(argv+1), "--no-filter");
 			no_filter = true;
 			break;
 		case OPT_debug:
@@ -6188,6 +6213,7 @@ static void parse_record_options(int argc,
 			ctx->filtered = 0;
 			break;
 		case OPT_tsyncinterval:
+			cmd_check_die(ctx, CMD_set, *(argv+1), "--tsync-interval");
 			top_instance.tsync.loop_interval = atoi(optarg);
 			guest_sync_set = true;
 			break;
@@ -6253,8 +6279,8 @@ static void parse_record_options(int argc,
 		}
 	}
 
-	if (do_ptrace && IS_START(ctx))
-		die("ptrace not supported with command start");
+	if (do_ptrace && (IS_START(ctx) || IS_CMDSET(ctx)))
+		die("ptrace not supported with command %s", *(argv+1));
 
 	for_all_instances(instance) {
 		if (instance->get_procmap && !instance->nr_filter_pids) {
@@ -6277,7 +6303,8 @@ static enum trace_type get_trace_cmd_type(enum trace_cmd cmd)
 		{CMD_extract, TRACE_TYPE_EXTRACT},
 		{CMD_profile, TRACE_TYPE_STREAM},
 		{CMD_start, TRACE_TYPE_START},
-		{CMD_record_agent, TRACE_TYPE_RECORD}
+		{CMD_record_agent, TRACE_TYPE_RECORD},
+		{CMD_set, TRACE_TYPE_SET}
 	};
 
 	for (int i = 0; i < ARRAY_SIZE(trace_type_per_command); i++) {
@@ -6354,8 +6381,10 @@ static void record_trace(int argc, char **argv,
 		ctx->topt = 1;
 
 	update_first_instance(ctx->instance, ctx->topt);
-	check_doing_something();
-	check_function_plugin();
+	if (!IS_CMDSET(ctx)) {
+		check_doing_something();
+		check_function_plugin();
+	}
 
 	if (!ctx->output)
 		ctx->output = DEFAULT_INPUT_FILE;
@@ -6383,7 +6412,8 @@ static void record_trace(int argc, char **argv,
 
 	if (!is_guest(ctx->instance))
 		fset = set_ftrace(!ctx->disable, ctx->total_disable);
-	tracecmd_disable_all_tracing(1);
+	if (!IS_CMDSET(ctx))
+		tracecmd_disable_all_tracing(1);
 
 	for_all_instances(instance)
 		set_clock(instance);
@@ -6435,9 +6465,11 @@ static void record_trace(int argc, char **argv,
 		bool wait_indefinitely = false;
 
 		update_task_filter();
-		tracecmd_enable_tracing();
 
-		if (type & TRACE_TYPE_START)
+		if (!IS_CMDSET(ctx))
+			tracecmd_enable_tracing();
+
+		if (type & (TRACE_TYPE_START | TRACE_TYPE_SET))
 			exit(0);
 
 		/* We don't ptrace ourself */
@@ -6498,6 +6530,15 @@ void trace_start(int argc, char **argv)
 	exit(0);
 }
 
+void trace_set(int argc, char **argv)
+{
+	struct common_record_context ctx;
+
+	parse_record_options(argc, argv, CMD_set, &ctx);
+	record_trace(argc, argv, &ctx);
+	exit(0);
+}
+
 void trace_extract(int argc, char **argv)
 {
 	struct common_record_context ctx;
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 58bca9e4..0c0e12f4 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -65,6 +65,41 @@ static struct usage_help usage_help[] = {
 		"               If 0 is specified, no loop is performed - timestamps offset is calculated only twice,"
 		"                                                         at the beginnig and at the end of the trace\n"
 	},
+	{
+		"set",
+		"set a ftarce configuration parameter",
+		" %s set [-v][-e event [-f filter]][-p plugin][-F][-d][-D] \\\n"
+		"           [-q][-s usecs][-O option ][-l func][-g func][-n func] \\\n"
+		"           [-P pid][-b size][-B buf][-m max][-C clock][command ...]\n"
+		"          -e enable event\n"
+		"          -f filter for previous -e event\n"
+		"          -R trigger for previous -e event\n"
+		"          -p set ftrace plugin\n"
+		"          -P set PIDs to be traced\n"
+		"          -c also trace the children of -F (or -P if kernel supports it)\n"
+		"          -C set the trace clock\n"
+		"          -T do a stacktrace on all events\n"
+		"          -l filter function name\n"
+		"          -g set graph function\n"
+		"          -n do not trace function\n"
+		"          -m max size per CPU in kilobytes\n"
+		"          -M set CPU mask to trace\n"
+		"          -v will negate all -e after it (disable those events)\n"
+		"          -d disable function tracer when running\n"
+		"          -D Full disable of function tracing (for all users)\n"
+		"          -O option to enable (or disable)\n"
+		"          -b change kernel buffersize (in kilobytes per CPU)\n"
+		"          -B create sub buffer and following events will be enabled here\n"
+		"          -i do not fail if an event is not found\n"
+		"          -q print no output to the screen\n"
+		"          --quiet print no output to the screen\n"
+		"          --module filter module name\n"
+		"          --func-stack perform a stack trace for function tracer\n"
+		"             (use with caution)\n"
+		"          --max-graph-depth limit function_graph depth\n"
+		"          --cmdlines-size change kernel saved_cmdlines_size\n"
+		"          --user execute the specified [command ...] as given user\n"
+	},
 	{
 		"start",
 		"start tracing without recording into a file",
-- 
2.26.2


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

* [PATCH v3 3/5] trace-cmd: Man page for "set" subcommand
  2020-06-03 12:15 [PATCH v3 0/5] New trace-cmd "set" command and changes in "start" Tzvetomir Stoyanov (VMware)
  2020-06-03 12:15 ` [PATCH v3 1/5] trace-cmd: Enable "trace-cmd start" to run a command Tzvetomir Stoyanov (VMware)
  2020-06-03 12:15 ` [PATCH v3 2/5] trace-cmd: Add new subcommand "set" Tzvetomir Stoyanov (VMware)
@ 2020-06-03 12:15 ` Tzvetomir Stoyanov (VMware)
  2020-06-04 15:50   ` Steven Rostedt
  2020-06-03 12:15 ` [PATCH v3 4/5] trace-cmd: Extend option "-v" to delete an ftrace instance Tzvetomir Stoyanov (VMware)
  2020-06-03 12:15 ` [PATCH v3 5/5] trace-cmd: Add description of "-G" option Tzvetomir Stoyanov (VMware)
  4 siblings, 1 reply; 11+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-06-03 12:15 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Man page, describing the new "trace-cmd set" subcommand

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 Documentation/trace-cmd-set.1.txt | 254 ++++++++++++++++++++++++++++++
 1 file changed, 254 insertions(+)
 create mode 100644 Documentation/trace-cmd-set.1.txt

diff --git a/Documentation/trace-cmd-set.1.txt b/Documentation/trace-cmd-set.1.txt
new file mode 100644
index 00000000..faf0b740
--- /dev/null
+++ b/Documentation/trace-cmd-set.1.txt
@@ -0,0 +1,254 @@
+TRACE-CMD-SET(1)
+================
+
+NAME
+----
+trace-cmd-set - set a configuration parameter of the Ftrace Linux internal tracer
+
+SYNOPSIS
+--------
+*trace-cmd set* ['OPTIONS'] ['command']
+
+DESCRIPTION
+-----------
+The trace-cmd(1) set command will set a configuration parameter of the Ftrace
+Linux kernel tracer. The specified *command* will be run after the ftrace state
+is set. The configured ftrace state can be restored to default
+using the trace-cmd-reset(1) command.
+
+OPTIONS
+-------
+*-p* 'tracer'::
+    Specify a tracer. Tracers usually do more than just trace an event.
+    Common tracers are: *function*, *function_graph*, *preemptirqsoff*,
+    *irqsoff*, *preemptoff* and *wakeup*. A tracer must be supported by the
+    running kernel. To see a list of available tracers, see trace-cmd-list(1).
+
+*-e* 'event'::
+    Specify an event to trace. Various static trace points have been added to
+    the Linux kernel. They are grouped by subsystem where you can enable all
+    events of a given subsystem or specify specific events to be enabled. The
+    'event' is of the format "subsystem:event-name". You can also just specify
+    the subsystem without the ':event-name' or the event-name without the
+    "subsystem:". Using "-e sched_switch" will enable the "sched_switch" event
+    where as, "-e sched" will enable all events under the "sched" subsystem.
+
+    The 'event' can also contain glob expressions. That is, "*stat*" will
+    select all events (or subsystems) that have the characters "stat" in their
+    names.
+
+    The keyword 'all' can be used to enable all events.
+
+*-T*::
+    Enable a stacktrace on each event. For example:
+
+          <idle>-0     [003] 58549.289091: sched_switch:         kworker/0:1:0 [120] R ==> trace-cmd:2603 [120]
+          <idle>-0     [003] 58549.289092: kernel_stack:         <stack trace>
+=> schedule (ffffffff814b260e)
+=> cpu_idle (ffffffff8100a38c)
+=> start_secondary (ffffffff814ab828)
+
+*--func-stack*::
+    Enable a stack trace on all functions. Note this is only applicable
+    for the "function" plugin tracer, and will only take effect if the
+    -l option is used and succeeds in limiting functions. If the function
+    tracer is not filtered, and the stack trace is enabled, you can live
+    lock the machine.
+
+*-f* 'filter'::
+    Specify a filter for the previous event. This must come after a *-e*. This
+    will filter what events get recorded based on the content of the event.
+    Filtering is passed to the kernel directly so what filtering is allowed
+    may depend on what version of the kernel you have. Basically, it will
+    let you use C notation to check if an event should be processed or not.
+
+----------------------------------------
+    ==, >=, <=, >, <, &, |, && and ||
+----------------------------------------
+
+    The above are usually safe to use to compare fields.
+
+*-R* 'trigger'::
+    Specify a trigger for the previous event. This must come after a *-e*.
+    This will add a given trigger to the given event. To only enable the trigger
+    and not the event itself, then place the event after the *-v* option.
+
+    See Documentation/trace/events.txt in the Linux kernel source for more
+    information on triggers.
+
+*-v*::
+    This will cause all events specified after it on the command line to not
+    be traced. This is useful for selecting a subsystem to be traced but to
+    leave out various events. For Example: "-e sched -v -e "\*stat\*"" will
+    enable all events in the sched subsystem except those that have "stat" in
+    their names.
+
+    Note: the *-v* option was taken from the way grep(1) inverts the following
+    matches.
+
+*-P* 'pid'::
+    This will filter only the specified process IDs. Using *-P* will let you
+    trace only events that are caused by the process.
+
+*-c*::
+     Used *-P* to trace the process' children too (if kernel supports it).
+
+*--user*::
+     Execute the specified *command* as given user.
+
+*-C* 'clock'::
+     Set the trace clock to "clock".
+
+     Use trace-cmd(1) list -C to see what clocks are available.
+
+*-l* 'function-name'::
+    This will limit the 'function' and 'function_graph' tracers to only trace
+    the given function name. More than one *-l* may be specified on the
+    command line to trace more than one function. The limited use of glob
+    expressions are also allowed. These are 'match\*' to only filter functions
+    that start with 'match'. '\*match' to only filter functions that end with
+    'match'. '\*match\*' to only filter on functions that contain 'match'.
+
+*-g* 'function-name'::
+    This option is for the function_graph plugin. It will graph the given
+    function. That is, it will only trace the function and all functions that
+    it calls. You can have more than one *-g* on the command line.
+
+*-n* 'function-name'::
+    This has the opposite effect of *-l*. The function given with the *-n*
+    option will not be traced. This takes precedence, that is, if you include
+    the same function for both *-n* and *-l*, it will not be traced.
+
+*-d*::
+    Some tracer plugins enable the function tracer by default. Like the
+    latency tracers. This option prevents the function tracer from being
+    enabled at start up.
+
+*-D*::
+    The option *-d* will try to use the function-trace option to disable the
+    function tracer (if available), otherwise it defaults to the proc file:
+    /proc/sys/kernel/ftrace_enabled, but will not touch it if the function-trace
+    option is available.  The *-D* option will disable both the ftrace_enabled
+    proc file as well as the function-trace option if it exists.
+
+    Note, this disable function tracing for all users, which includes users
+    outside of ftrace tracers (stack_tracer, perf, etc).
+
+*-O* 'option'::
+    Ftrace has various options that can be enabled or disabled. This allows
+    you to set them. Appending the text 'no' to an option disables it.
+    For example: "-O nograph-time" will disable the "graph-time" Ftrace
+    option.
+
+*-b* 'size'::
+    This sets the ring buffer size to 'size' kilobytes. Because the Ftrace
+    ring buffer is per CPU, this size is the size of each per CPU ring buffer
+    inside the kernel. Using "-b 10000" on a machine with 4 CPUs will make
+    Ftrace have a total buffer size of 40 Megs.
+
+*-B* 'buffer-name'::
+    If the kernel supports multiple buffers, this will add a buffer with
+    the given name. If the buffer name already exists, that buffer is just
+    reset.
+
+    After a buffer name is stated, all events added after that will be
+    associated with that buffer. If no buffer is specified, or an event
+    is specified before a buffer name, it will be associated with the
+    main (toplevel) buffer.
+
+     trace-cmd set -e sched -B block -e block -B time -e timer sleep 1
+
+    The above is will enable all sched events in the main buffer. It will
+    then create a 'block' buffer instance and enable all block events within
+    that buffer. A 'time' buffer instance is created and all timer events
+    will be enabled for that event.
+
+*-m* 'size'::
+    The max size in kilobytes that a per cpu buffer should be. Note, due
+    to rounding to page size, the number may not be totally correct.
+    Also, this is performed by switching between two buffers that are half
+    the given size thus the output may not be of the given size even if
+    much more was written.
+
+    Use this to prevent running out of diskspace for long runs.
+
+*-M* 'cpumask'::
+    Set the cpumask for to trace. It only affects the last buffer instance
+    given. If supplied before any buffer instance, then it affects the
+    main buffer. The value supplied must be a hex number.
+
+     trace-cmd set -p function -M c -B events13 -e all -M 5
+
+    If the -M is left out, then the mask stays the same. To enable all
+    CPUs, pass in a value of '-1'.
+
+*-i*::
+    By default, if an event is listed that trace-cmd does not find, it
+    will exit with an error. This option will just ignore events that are
+    listed on the command line but are not found on the system.
+
+*-q* | *--quiet*::
+    Suppresses normal output, except for errors.
+
+*--max-graph-depth* 'depth'::
+    Set the maximum depth the function_graph tracer will trace into a function.
+    A value of one will only show where userspace enters the kernel but not any
+    functions called in the kernel. The default is zero, which means no limit.
+
+*--cmdlines-size* 'size'::
+    Set the number of entries the kernel tracing file "saved_cmdlines" can
+    contain. This file is a circular buffer which stores the mapping between
+    cmdlines and PIDs. If full, it leads to unresolved cmdlines ("<...>") within
+    the trace. The kernel default value is 128.
+
+*--module* 'module'::
+    Filter a module's name in function tracing. It is equivalent to adding
+    ':mod:module' after all other functions being filtered. If no other function
+    filter is listed, then all modules functions will be filtered in the filter.
+
+    '--module snd'  is equivalent to  '-l :mod:snd'
+
+    '--module snd -l "*jack*"' is equivalent to '-l "*jack*:mod:snd"'
+
+    '--module snd -n "*"' is equivalent to '-n :mod:snd'
+
+*--stderr*::
+    Have output go to stderr instead of stdout, but the output of the command
+    executed will not be changed. This is useful if you want to monitor the
+    output of the command being executed, but not see the output from trace-cmd.
+
+EXAMPLES
+--------
+
+Enable all events for tracing:
+
+------------------------------
+ # trace-cmd set -e all
+------------------------------
+
+Set the function tracer:
+
+------------------------------
+ # trace-cmd set -p function
+------------------------------
+
+
+SEE ALSO
+--------
+trace-cmd(1), trace-cmd-report(1), trace-cmd-start(1), trace-cmd-stop(1),
+trace-cmd-extract(1), trace-cmd-reset(1), trace-cmd-split(1),
+trace-cmd-list(1), trace-cmd-listen(1), trace-cmd-profile(1)
+
+AUTHOR
+------
+Written by Steven Rostedt, <rostedt@goodmis.org>
+
+RESOURCES
+---------
+git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/trace-cmd.git
+
+COPYING
+-------
+Copyright \(C) 2010 Red Hat, Inc. Free use of this software is granted under
+the terms of the GNU Public License (GPL).
+
-- 
2.26.2


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

* [PATCH v3 4/5] trace-cmd: Extend option "-v" to delete an ftrace instance
  2020-06-03 12:15 [PATCH v3 0/5] New trace-cmd "set" command and changes in "start" Tzvetomir Stoyanov (VMware)
                   ` (2 preceding siblings ...)
  2020-06-03 12:15 ` [PATCH v3 3/5] trace-cmd: Man page for "set" subcommand Tzvetomir Stoyanov (VMware)
@ 2020-06-03 12:15 ` Tzvetomir Stoyanov (VMware)
  2020-06-03 12:15 ` [PATCH v3 5/5] trace-cmd: Add description of "-G" option Tzvetomir Stoyanov (VMware)
  4 siblings, 0 replies; 11+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-06-03 12:15 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Extended the "trace-cmd set" option "-v" to delete the instance specified
after it on the command line with "-B". For example:
 trace-cmd set -v -B bar -B foo
will delete instance bar and create a new instance foo.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 Documentation/trace-cmd-set.1.txt | 19 +++++++----
 tracecmd/include/trace-local.h    |  1 +
 tracecmd/trace-record.c           | 55 ++++++++++++++++++++++++++++---
 tracecmd/trace-usage.c            |  4 +--
 4 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/Documentation/trace-cmd-set.1.txt b/Documentation/trace-cmd-set.1.txt
index faf0b740..af1f69c6 100644
--- a/Documentation/trace-cmd-set.1.txt
+++ b/Documentation/trace-cmd-set.1.txt
@@ -77,15 +77,20 @@ OPTIONS
     information on triggers.
 
 *-v*::
-    This will cause all events specified after it on the command line to not
-    be traced. This is useful for selecting a subsystem to be traced but to
-    leave out various events. For Example: "-e sched -v -e "\*stat\*"" will
-    enable all events in the sched subsystem except those that have "stat" in
-    their names.
-
+    This will negate options specified after it on the command line. It affects:
+[verse]
+--
+     *-e*: Causes all specified events to not be traced. This is useful for
+           selecting a subsystem to be traced but to leave out various events.
+           For example: "-e sched -v -e "\*stat\*"" will enable all events in
+           the sched subsystem except those that have "stat" in their names.
+     *-B*: Deletes the specified ftrace instance. There must be no
+           configuration options related to this instance in the command line.
+           For example: "-v -B bar -B foo" will delete instance bar and create
+           a new instance foo.
     Note: the *-v* option was taken from the way grep(1) inverts the following
     matches.
-
+--
 *-P* 'pid'::
     This will filter only the specified process IDs. Using *-P* will let you
     trace only events that are caused by the process.
diff --git a/tracecmd/include/trace-local.h b/tracecmd/include/trace-local.h
index 5ec05149..d148aa16 100644
--- a/tracecmd/include/trace-local.h
+++ b/tracecmd/include/trace-local.h
@@ -205,6 +205,7 @@ struct buffer_instance {
 	char			*output_file;
 	struct event_list	*events;
 	struct event_list	**event_next;
+	bool			delete;
 
 	struct event_list	*sched_switch_event;
 	struct event_list	*sched_wakeup_event;
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index d5515c52..ab085f80 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -5797,6 +5797,27 @@ static inline void cmd_check_die(struct common_record_context *ctx,
 		    "Did you mean 'record'?", param, cmd);
 }
 
+static inline void remove_instances(struct buffer_instance *instances)
+{
+	struct buffer_instance *del;
+
+	while (instances) {
+		del = instances;
+		instances = instances->next;
+		tracefs_instance_destroy(del->tracefs);
+		tracefs_instance_free(del->tracefs);
+		free(del);
+	}
+}
+
+static inline void
+check_instance_die(struct buffer_instance *instance, char *param)
+{
+	if (instance->delete)
+		die("Instance %s is marked for deletion, invalid option %s",
+		    tracefs_instance_get_name(instance->tracefs), param);
+}
+
 static void parse_record_options(int argc,
 				 char **argv,
 				 enum trace_cmd curr_cmd,
@@ -5810,8 +5831,8 @@ static void parse_record_options(int argc,
 	char *pid;
 	char *sav;
 	int name_counter = 0;
-	int neg_event = 0;
-	struct buffer_instance *instance;
+	int negative = 0;
+	struct buffer_instance *instance, *del_list = NULL;
 	bool guest_sync_set = false;
 	int do_children = 0;
 	int fpids_count = 0;
@@ -5881,6 +5902,7 @@ static void parse_record_options(int argc,
 			}
 			break;
 		case 'e':
+			check_instance_die(ctx->instance, "-e");
 			ctx->events = 1;
 			event = malloc(sizeof(*event));
 			if (!event)
@@ -5888,7 +5910,7 @@ static void parse_record_options(int argc,
 			memset(event, 0, sizeof(*event));
 			event->event = optarg;
 			add_event(ctx->instance, event);
-			event->neg = neg_event;
+			event->neg = negative;
 			event->filter = NULL;
 			last_event = event;
 
@@ -5954,6 +5976,7 @@ static void parse_record_options(int argc,
 			ctx->global = 1;
 			break;
 		case 'P':
+			check_instance_die(ctx->instance, "-P");
 			test_set_event_pid(ctx->instance);
 			pids = strdup(optarg);
 			if (!pids)
@@ -5969,6 +5992,7 @@ static void parse_record_options(int argc,
 			free(pids);
 			break;
 		case 'c':
+			check_instance_die(ctx->instance, "-c");
 			test_set_event_pid(ctx->instance);
 			do_children = 1;
 			if (!ctx->instance->have_event_fork) {
@@ -5985,13 +6009,14 @@ static void parse_record_options(int argc,
 				save_option(ctx->instance, "function-fork");
 			break;
 		case 'C':
+			check_instance_die(ctx->instance, "-C");
 			ctx->instance->clock = optarg;
 			ctx->instance->flags |= BUFFER_FL_HAS_CLOCK;
 			if (is_top_instance(ctx->instance))
 				guest_sync_set = true;
 			break;
 		case 'v':
-			neg_event = 1;
+			negative = 1;
 			break;
 		case 'l':
 			add_func(&ctx->instance->filter_funcs,
@@ -5999,15 +6024,18 @@ static void parse_record_options(int argc,
 			ctx->filtered = 1;
 			break;
 		case 'n':
+			check_instance_die(ctx->instance, "-n");
 			add_func(&ctx->instance->notrace_funcs,
 				 ctx->instance->filter_mod, optarg);
 			ctx->filtered = 1;
 			break;
 		case 'g':
+			check_instance_die(ctx->instance, "-g");
 			add_func(&graph_funcs, ctx->instance->filter_mod, optarg);
 			ctx->filtered = 1;
 			break;
 		case 'p':
+			check_instance_die(ctx->instance, "-p");
 			if (ctx->instance->plugin)
 				die("only one plugin allowed");
 			for (plugin = optarg; isspace(*plugin); plugin++)
@@ -6057,14 +6085,17 @@ static void parse_record_options(int argc,
 			}
 			break;
 		case 'O':
+			check_instance_die(ctx->instance, "-O");
 			option = optarg;
 			save_option(ctx->instance, option);
 			break;
 		case 'T':
+			check_instance_die(ctx->instance, "-T");
 			save_option(ctx->instance, "stacktrace");
 			break;
 		case 'H':
 			cmd_check_die(ctx, CMD_set, *(argv+1), "-H");
+			check_instance_die(ctx->instance, "-H");
 			add_hook(ctx->instance, optarg);
 			ctx->events = 1;
 			break;
@@ -6109,6 +6140,7 @@ static void parse_record_options(int argc,
 			max_kb = atoi(optarg);
 			break;
 		case 'M':
+			check_instance_die(ctx->instance, "-M");
 			ctx->instance->cpumask = alloc_mask_from_hex(ctx->instance, optarg);
 			break;
 		case 't':
@@ -6119,13 +6151,20 @@ static void parse_record_options(int argc,
 				use_tcp = 1;
 			break;
 		case 'b':
+			check_instance_die(ctx->instance, "-b");
 			ctx->instance->buffer_size = atoi(optarg);
 			break;
 		case 'B':
 			ctx->instance = create_instance(optarg);
 			if (!ctx->instance)
 				die("Failed to create instance");
-			add_instance(ctx->instance, local_cpu_count);
+			ctx->instance->delete = negative;
+			negative = 0;
+			if (ctx->instance->delete) {
+				ctx->instance->next = del_list;
+				del_list = ctx->instance;
+			} else
+				add_instance(ctx->instance, local_cpu_count);
 			if (IS_PROFILE(ctx))
 				ctx->instance->flags |= BUFFER_FL_PROFILE;
 			break;
@@ -6144,6 +6183,7 @@ static void parse_record_options(int argc,
 		case OPT_procmap:
 			cmd_check_die(ctx, CMD_start, *(argv+1), "--proc-map");
 			cmd_check_die(ctx, CMD_set, *(argv+1), "--proc-map");
+			check_instance_die(ctx->instance, "--proc-map");
 			ctx->instance->get_procmap = 1;
 			break;
 		case OPT_date:
@@ -6166,6 +6206,7 @@ static void parse_record_options(int argc,
 			break;
 		case OPT_profile:
 			cmd_check_die(ctx, CMD_set, *(argv+1), "--profile");
+			check_instance_die(ctx->instance, "--profile");
 			handle_init = trace_init_profile;
 			ctx->instance->flags |= BUFFER_FL_PROFILE;
 			ctx->events = 1;
@@ -6190,6 +6231,7 @@ static void parse_record_options(int argc,
 			ctx->data_flags |= DATA_FL_OFFSET;
 			break;
 		case OPT_max_graph_depth:
+			check_instance_die(ctx->instance, "--max-graph-depth");
 			free(ctx->instance->max_graph_depth);
 			ctx->instance->max_graph_depth = strdup(optarg);
 			if (!ctx->instance->max_graph_depth)
@@ -6206,6 +6248,7 @@ static void parse_record_options(int argc,
 			tracecmd_set_debug(true);
 			break;
 		case OPT_module:
+			check_instance_die(ctx->instance, "--module");
 			if (ctx->instance->filter_mod)
 				add_func(&ctx->instance->filter_funcs,
 					 ctx->instance->filter_mod, "*");
@@ -6226,6 +6269,8 @@ static void parse_record_options(int argc,
 		}
 	}
 
+	remove_instances(del_list);
+
 	/* If --date is specified, prepend it to all guest VM flags */
 	if (ctx->date) {
 		struct buffer_instance *instance;
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 0c0e12f4..96647a1e 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -34,7 +34,7 @@ static struct usage_help usage_help[] = {
 		"          -n do not trace function\n"
 		"          -m max size per CPU in kilobytes\n"
 		"          -M set CPU mask to trace\n"
-		"          -v will negate all -e after it (disable those events)\n"
+		"          -v will negate all -e (disable those events) and -B (delete those instances) after it\n"
 		"          -d disable function tracer when running\n"
 		"          -D Full disable of function tracing (for all users)\n"
 		"          -o data output file [default trace.dat]\n"
@@ -84,7 +84,7 @@ static struct usage_help usage_help[] = {
 		"          -n do not trace function\n"
 		"          -m max size per CPU in kilobytes\n"
 		"          -M set CPU mask to trace\n"
-		"          -v will negate all -e after it (disable those events)\n"
+		"          -v will negate all -e (disable those events) and -B (delete those instances) after it\n"
 		"          -d disable function tracer when running\n"
 		"          -D Full disable of function tracing (for all users)\n"
 		"          -O option to enable (or disable)\n"
-- 
2.26.2


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

* [PATCH v3 5/5] trace-cmd: Add description of "-G" option
  2020-06-03 12:15 [PATCH v3 0/5] New trace-cmd "set" command and changes in "start" Tzvetomir Stoyanov (VMware)
                   ` (3 preceding siblings ...)
  2020-06-03 12:15 ` [PATCH v3 4/5] trace-cmd: Extend option "-v" to delete an ftrace instance Tzvetomir Stoyanov (VMware)
@ 2020-06-03 12:15 ` Tzvetomir Stoyanov (VMware)
  2020-06-04 15:51   ` Steven Rostedt
  4 siblings, 1 reply; 11+ messages in thread
From: Tzvetomir Stoyanov (VMware) @ 2020-06-03 12:15 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel

Documented "trace-cmd record -G" option in man page and in
the usage output.

Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
---
 Documentation/trace-cmd-record.1.txt | 4 ++++
 tracecmd/trace-usage.c               | 1 +
 2 files changed, 5 insertions(+)

diff --git a/Documentation/trace-cmd-record.1.txt b/Documentation/trace-cmd-record.1.txt
index f5ecad5c..99d1a156 100644
--- a/Documentation/trace-cmd-record.1.txt
+++ b/Documentation/trace-cmd-record.1.txt
@@ -312,6 +312,10 @@ OPTIONS
 
     See trace-cmd-profile(1) for more details and examples.
 
+*-G*::
+    Set interrupt (soft and hard) events as global (associated to CPU
+    instead of tasks). Only works for --profile.
+
 *-H* 'event-hooks'::
     Add custom event matching to connect any two events together. When not
     used with *--profile*, it will save the parameter and this will be
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 96647a1e..8036ac7b 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -53,6 +53,7 @@ static struct usage_help usage_help[] = {
 		"          --module filter module name\n"
 		"          --by-comm used with --profile, merge events for related comms\n"
 		"          --profile enable tracing options needed for report --profile\n"
+		"          -G when profiling, set soft and hard irqs as global\n"
 		"          --func-stack perform a stack trace for function tracer\n"
 		"             (use with caution)\n"
 		"          --max-graph-depth limit function_graph depth\n"
-- 
2.26.2


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

* Re: [PATCH v3 3/5] trace-cmd: Man page for "set" subcommand
  2020-06-03 12:15 ` [PATCH v3 3/5] trace-cmd: Man page for "set" subcommand Tzvetomir Stoyanov (VMware)
@ 2020-06-04 15:50   ` Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2020-06-04 15:50 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Wed,  3 Jun 2020 15:15:04 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> +AUTHOR
> +------
> +Written by Steven Rostedt, <rostedt@goodmis.org>

You can add your name as well. ;-)

> +
> +RESOURCES
> +---------
> +git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/trace-cmd.git
> +
> +COPYING
> +-------
> +Copyright \(C) 2010 Red Hat, Inc. Free use of this software is granted under
> +the terms of the GNU Public License (GPL).
> +

Remove the last extra line.

Thanks!

-- Steve

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

* Re: [PATCH v3 5/5] trace-cmd: Add description of "-G" option
  2020-06-03 12:15 ` [PATCH v3 5/5] trace-cmd: Add description of "-G" option Tzvetomir Stoyanov (VMware)
@ 2020-06-04 15:51   ` Steven Rostedt
  2020-06-08 21:42     ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2020-06-04 15:51 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Wed,  3 Jun 2020 15:15:06 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> --- a/tracecmd/trace-usage.c
> +++ b/tracecmd/trace-usage.c
> @@ -53,6 +53,7 @@ static struct usage_help usage_help[] = {
>  		"          --module filter module name\n"
>  		"          --by-comm used with --profile, merge events for related comms\n"
>  		"          --profile enable tracing options needed for report --profile\n"
> +		"          -G when profiling, set soft and hard irqs as global\n"

This should be in with the other "simple" options (those with a single
character). The long options "--foo" should always be at the end after all
simple ones.

Thanks!

-- Steve

>  		"          --func-stack perform a stack trace for function tracer\n"
>  		"             (use with caution)\n"
>  		"          --max-graph-depth limit function_graph depth\n"

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

* Re: [PATCH v3 1/5] trace-cmd: Enable "trace-cmd start" to run a command
  2020-06-03 12:15 ` [PATCH v3 1/5] trace-cmd: Enable "trace-cmd start" to run a command Tzvetomir Stoyanov (VMware)
@ 2020-06-08 19:35   ` Steven Rostedt
  2020-06-08 20:39     ` [PATCH] trace-cmd: Have trace-cmd start wait for command unless --fork is specified Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2020-06-08 19:35 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Wed,  3 Jun 2020 15:15:02 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> There is a resrtiction of "trace-cmd start" which does not allow it to
> run a command, specified by "-F" option or at the end of the command
> line. However, there are use cases where this is useful.
> The resrtiction is removed, it allows "trace-cmd start" to run commands
> just like "trace-cmd record".
> 
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  Documentation/trace-cmd-start.1.txt |  2 +-
>  tracecmd/trace-record.c             | 28 ++++++++++++++++++++--------
>  tracecmd/trace-usage.c              |  2 +-
>  3 files changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/trace-cmd-start.1.txt b/Documentation/trace-cmd-start.1.txt
> index 2b027b01..63d2034c 100644
> --- a/Documentation/trace-cmd-start.1.txt
> +++ b/Documentation/trace-cmd-start.1.txt
> @@ -21,7 +21,7 @@ system or can be extracted with trace-cmd-extract(1).
>  OPTIONS
>  -------
>  The options are the same as 'trace-cmd-record(1)', except that it does not
> -take options specific to recording (*-s*, *-o*, *-F*, *-N*, and *-t*).
> +take options specific to recording (*-s*, *-o*, *-N*, and *-t*).
>  
>  SEE ALSO
>  --------
> diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
> index 3242368e..f345c6d6 100644
> --- a/tracecmd/trace-record.c
> +++ b/tracecmd/trace-record.c
> @@ -1598,7 +1598,8 @@ static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
>  		/* child */
>  		update_task_filter();
>  		tracecmd_enable_tracing();
> -		enable_ptrace();
> +		if (type != TRACE_TYPE_START)
> +			enable_ptrace();
>  		/*
>  		 * If we are using stderr for stdout, switch
>  		 * it back to the saved stdout for the code we run.
> @@ -1619,6 +1620,8 @@ static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
>  			die("Failed to exec %s", argv[0]);
>  		}
>  	}
> +	if (type & TRACE_TYPE_START)
> +		exit(0);

After playing with this a bit, I think we should not just fork and
ignore the executable. We should wait for it to finish. This means we
can be able to ptrace via trace-cmd start as well.

The reason I say this, is the few times I used it, I didn't think it
was working. For example, I did:

 # trace-cmd start -e all ls -ltr /bin

And it returned then printed the output of ls. Because of that, it
looked like it hung:

[root@bxtest ~]# trace-cmd start -e all ls -ltr ~/bin
[root@bxtest ~]# total 48716
-rwxr-xr-x 1 root root    50320 Apr 24 18:12 bootconfig
-rwxr-xr-x 3 root root 24902584 May 27 12:31 trace
-rwxr-xr-x 3 root root 24902584 May 27 12:31 perf
-rwxr-xr-x 1 root root    20860 May 27 12:31 perf-read-vdso32

And I'm waiting there for my prompt, not noticing that the prompt was
already printed before the output.

Although, we are not recording, it should by default wait for the
execed program to finish. We can add an option to change that, like:

 # trace-cmd start --fork -e all ls -ltr ~/bin

Which would let the user to explicitly ask for this behavior. But it
shouldn't be the default behavior. I basically think it hangs each time
I do this, even though I know why it does this.

-- Steve




>  	if (do_ptrace) {
>  		ptrace_attach(NULL, pid);
>  		ptrace_wait(type);
> @@ -5782,6 +5785,14 @@ static void add_arg(struct buffer_instance *instance,
>  	/* Not found? */
>  }
>  
> +static inline void cmd_check_die(struct common_record_context *ctx,
> +				 enum trace_cmd id, char *cmd, char *param)
> +{
> +	if (ctx->curr_cmd == id)
> +		die("%s has no effect with the command %s\n"
> +		    "Did you mean 'record'?", param, cmd);
> +}
> +
>  static void parse_record_options(int argc,
>  				 char **argv,
>  				 enum trace_cmd curr_cmd,
> @@ -6114,6 +6125,7 @@ static void parse_record_options(int argc,
>  				die("Failed to allocate user name");
>  			break;
>  		case OPT_procmap:
> +			cmd_check_die(ctx, CMD_start, *(argv+1), "--proc-map");
>  			ctx->instance->get_procmap = 1;
>  			break;
>  		case OPT_date:
> @@ -6221,9 +6233,6 @@ static void parse_record_options(int argc,
>  		die(" -c can only be used with -F (or -P with event-fork support)");
>  
>  	if ((argc - optind) >= 2) {
> -		if (IS_START(ctx))
> -			die("Command start does not take any commands\n"
> -			    "Did you mean 'record'?");
>  		if (IS_EXTRACT(ctx))
>  			die("Command extract does not take any commands\n"
>  			    "Did you mean 'record'?");
> @@ -6244,6 +6253,9 @@ static void parse_record_options(int argc,
>  		}
>  	}
>  
> +	if (do_ptrace && IS_START(ctx))
> +		die("ptrace not supported with command start");
> +
>  	for_all_instances(instance) {
>  		if (instance->get_procmap && !instance->nr_filter_pids) {
>  			warning("--proc-map is ignored for instance %s, "
> @@ -6410,10 +6422,6 @@ static void record_trace(int argc, char **argv,
>  		signal(SIGINT, finish);
>  		if (!latency)
>  			start_threads(type, ctx);
> -	} else {
> -		update_task_filter();
> -		tracecmd_enable_tracing();
> -		exit(0);
>  	}
>  
>  	if (ctx->run_command) {
> @@ -6428,6 +6436,10 @@ static void record_trace(int argc, char **argv,
>  
>  		update_task_filter();
>  		tracecmd_enable_tracing();
> +
> +		if (type & TRACE_TYPE_START)
> +			exit(0);
> +
>  		/* We don't ptrace ourself */
>  		if (do_ptrace) {
>  			for_all_instances(instance) {
> diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
> index d43ca490..58bca9e4 100644
> --- a/tracecmd/trace-usage.c
> +++ b/tracecmd/trace-usage.c
> @@ -69,7 +69,7 @@ static struct usage_help usage_help[] = {
>  		"start",
>  		"start tracing without recording into a file",
>  		" %s start [-e event][-p plugin][-d][-O option ][-P pid]\n"
> -		"          Uses same options as record, but does not run a command.\n"
> +		"          Uses same options as record.\n"
>  		"          It only enables the tracing and exits\n"
>  	},
>  	{


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

* [PATCH] trace-cmd: Have trace-cmd start wait for command unless --fork is specified
  2020-06-08 19:35   ` Steven Rostedt
@ 2020-06-08 20:39     ` Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2020-06-08 20:39 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

[ I just realized I already pushed your patch. Here's an update to what
  I would like. ]

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Instead of forking a command specified on the command line and returning
right away, have it wait for the process to finish before returning. Unless
"--fork" is specified as well.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 Documentation/trace-cmd-start.1.txt |  7 +++++++
 tracecmd/trace-record.c             | 18 +++++++++++++-----
 tracecmd/trace-usage.c              |  2 ++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/Documentation/trace-cmd-start.1.txt b/Documentation/trace-cmd-start.1.txt
index 63d2034c..07f2ed3a 100644
--- a/Documentation/trace-cmd-start.1.txt
+++ b/Documentation/trace-cmd-start.1.txt
@@ -23,6 +23,13 @@ OPTIONS
 The options are the same as 'trace-cmd-record(1)', except that it does not
 take options specific to recording (*-s*, *-o*, *-N*, and *-t*).
 
+*--fork* ::
+   This option is only available for trace-cmd start. It tells trace-cmd
+   to not wait for the process to finish before returning.
+   With this option, trace-cmd start will return right after it forks
+   the process on the command line. This option only has an effect if
+   trace-cmd start also executes a command.
+
 SEE ALSO
 --------
 trace-cmd(1), trace-cmd-record(1), trace-cmd-report(1), trace-cmd-stop(1),
diff --git a/tracecmd/trace-record.c b/tracecmd/trace-record.c
index 4d2039a1..143b736e 100644
--- a/tracecmd/trace-record.c
+++ b/tracecmd/trace-record.c
@@ -86,6 +86,8 @@ static char *host;
 
 static bool quiet;
 
+static bool fork_process;
+
 /* Max size to let a per cpu file get */
 static int max_kb;
 
@@ -1604,7 +1606,7 @@ static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
 		/* child */
 		update_task_filter();
 		tracecmd_enable_tracing();
-		if (type != TRACE_TYPE_START)
+		if (!fork_process)
 			enable_ptrace();
 		/*
 		 * If we are using stderr for stdout, switch
@@ -1626,13 +1628,15 @@ static void run_cmd(enum trace_type type, const char *user, int argc, char **arg
 			die("Failed to exec %s", argv[0]);
 		}
 	}
-	if (type & TRACE_TYPE_START)
+	if (fork_process)
 		exit(0);
 	if (do_ptrace) {
 		ptrace_attach(NULL, pid);
 		ptrace_wait(type);
 	} else
 		trace_waitpid(type, pid, &status, 0);
+	if (type & TRACE_TYPE_START)
+		exit(0);
 }
 
 static void
@@ -5535,6 +5539,7 @@ void init_top_instance(void)
 }
 
 enum {
+	OPT_fork		= 241,
 	OPT_tsyncinterval	= 242,
 	OPT_user		= 243,
 	OPT_procmap		= 244,
@@ -5851,6 +5856,7 @@ static void parse_record_options(int argc,
 			{"user", required_argument, NULL, OPT_user},
 			{"module", required_argument, NULL, OPT_module},
 			{"tsync-interval", required_argument, NULL, OPT_tsyncinterval},
+			{"fork", no_argument, NULL, OPT_fork},
 			{NULL, 0, NULL, 0}
 		};
 
@@ -6204,6 +6210,11 @@ static void parse_record_options(int argc,
 			top_instance.tsync.loop_interval = atoi(optarg);
 			guest_sync_set = true;
 			break;
+		case OPT_fork:
+			if (!IS_START(ctx))
+				die("--fork option used for 'start' command only");
+			fork_process = true;
+			break;
 		case OPT_quiet:
 		case 'q':
 			quiet = true;
@@ -6266,9 +6277,6 @@ static void parse_record_options(int argc,
 		}
 	}
 
-	if (do_ptrace && IS_START(ctx))
-		die("ptrace not supported with command start");
-
 	for_all_instances(instance) {
 		if (instance->get_procmap && !instance->nr_filter_pids) {
 			warning("--proc-map is ignored for instance %s, "
diff --git a/tracecmd/trace-usage.c b/tracecmd/trace-usage.c
index 58bca9e4..1e832be8 100644
--- a/tracecmd/trace-usage.c
+++ b/tracecmd/trace-usage.c
@@ -71,6 +71,8 @@ static struct usage_help usage_help[] = {
 		" %s start [-e event][-p plugin][-d][-O option ][-P pid]\n"
 		"          Uses same options as record.\n"
 		"          It only enables the tracing and exits\n"
+		"\n"
+		"        --fork: If a command is specified, then return right after it forks\n"
 	},
 	{
 		"extract",
-- 
2.25.4


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

* Re: [PATCH v3 5/5] trace-cmd: Add description of "-G" option
  2020-06-04 15:51   ` Steven Rostedt
@ 2020-06-08 21:42     ` Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2020-06-08 21:42 UTC (permalink / raw)
  To: Tzvetomir Stoyanov (VMware); +Cc: linux-trace-devel

On Thu, 4 Jun 2020 11:51:40 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> > @@ -53,6 +53,7 @@ static struct usage_help usage_help[] = {
> >  		"          --module filter module name\n"
> >  		"          --by-comm used with --profile, merge events for related comms\n"
> >  		"          --profile enable tracing options needed for report --profile\n"
> > +		"          -G when profiling, set soft and hard irqs as global\n"  
> 
> This should be in with the other "simple" options (those with a single
> character). The long options "--foo" should always be at the end after all
> simple ones.

Let me add. I see why you did it this way (because it is right under
the --profile), but I think it still should be with the other simple
options.

I'll update it.

Thanks!

-- Steve

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

end of thread, other threads:[~2020-06-08 21:42 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 12:15 [PATCH v3 0/5] New trace-cmd "set" command and changes in "start" Tzvetomir Stoyanov (VMware)
2020-06-03 12:15 ` [PATCH v3 1/5] trace-cmd: Enable "trace-cmd start" to run a command Tzvetomir Stoyanov (VMware)
2020-06-08 19:35   ` Steven Rostedt
2020-06-08 20:39     ` [PATCH] trace-cmd: Have trace-cmd start wait for command unless --fork is specified Steven Rostedt
2020-06-03 12:15 ` [PATCH v3 2/5] trace-cmd: Add new subcommand "set" Tzvetomir Stoyanov (VMware)
2020-06-03 12:15 ` [PATCH v3 3/5] trace-cmd: Man page for "set" subcommand Tzvetomir Stoyanov (VMware)
2020-06-04 15:50   ` Steven Rostedt
2020-06-03 12:15 ` [PATCH v3 4/5] trace-cmd: Extend option "-v" to delete an ftrace instance Tzvetomir Stoyanov (VMware)
2020-06-03 12:15 ` [PATCH v3 5/5] trace-cmd: Add description of "-G" option Tzvetomir Stoyanov (VMware)
2020-06-04 15:51   ` Steven Rostedt
2020-06-08 21:42     ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).