Subject: [PATCH v2 00/10] trace-cmd: Refactoring trace_record()

The following series of patches is a refactoring of trace_record() trying to
reduce as much as possible its complexity but, at the same time, without making
risky changes. All the patches are relatively small and potentially no-brainer
steps towards the final shape of the code.

The rationale for this effort is to make the code much easier to maintain.

Vladislav Valtchev (VMware) (10):
trace-cmd: Extract parse_record_options() from trace_record()
trace-cmd: Replacing cmd flags w/ a trace_cmd enum
trace-cmd: Extracting record_trace()
trace-cmd: Rename trace_profile() to do_trace_profile()
trace-cmd: Making start,extract,stream,profile separate funcs
trace-cmd: Extr. profile-specific code from record_trace
trace-cmd: Mov init_common_record_context in parse_record_options
trace-cmd: Introducing get_trace_cmd_type()
trace-cmd: Extract finalize_record_trace()
trace-cmd: Fork record_trace() for the extract case

trace-cmd.c | 8 +-
trace-local.h | 10 +-
trace-profile.c | 2 +-
trace-read.c | 2 +-
trace-record.c | 640 +++++++++++++++++++++++++++++++++-----------------------
5 files changed, 390 insertions(+), 272 deletions(-)

--
2.14.1


From 1586130470606637727@xxx Thu Dec 07 13:04:52 +0000 2017
X-GM-THRID: 1585986712478089735
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread


Subject: [PATCH v2 07/10] trace-cmd: Mov init_common_record_context in parse_record_options

This short patch moves init_common_record_context() into parse_record_option()
after checking that no trace_* function in trace-record.c really needs to do
work after init_common_record_context() but before parse_record_option().
In particular, two statements in trace_profile() have been moved after the
call of parse_record_option(), since they did not actually affect it.

Signed-off-by: Vladislav Valtchev (VMware) <[email protected]>
---
trace-record.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index 55e81cf..7688565 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4405,6 +4405,7 @@ static void init_common_record_context(struct common_record_context *ctx,

static void parse_record_options(int argc,
char **argv,
+ enum trace_cmd curr_cmd,
struct common_record_context *ctx)
{
const char *plugin = NULL;
@@ -4416,6 +4417,8 @@ static void parse_record_options(int argc,
char *sav;
int neg_event = 0;

+ init_common_record_context(ctx, curr_cmd);
+
for (;;) {
int option_index = 0;
int ret;
@@ -4925,8 +4928,7 @@ void trace_start(int argc, char **argv)
{
struct common_record_context ctx;

- init_common_record_context(&ctx, CMD_start);
- parse_record_options(argc, argv, &ctx);
+ parse_record_options(argc, argv, CMD_start, &ctx);
record_trace(argc, argv, &ctx);
exit(0);
}
@@ -4935,8 +4937,7 @@ void trace_extract(int argc, char **argv)
{
struct common_record_context ctx;

- init_common_record_context(&ctx, CMD_extract);
- parse_record_options(argc, argv, &ctx);
+ parse_record_options(argc, argv, CMD_extract, &ctx);
record_trace(argc, argv, &ctx);
exit(0);
}
@@ -4945,8 +4946,7 @@ void trace_stream(int argc, char **argv)
{
struct common_record_context ctx;

- init_common_record_context(&ctx, CMD_stream);
- parse_record_options(argc, argv, &ctx);
+ parse_record_options(argc, argv, CMD_stream, &ctx);
record_trace(argc, argv, &ctx);
exit(0);
}
@@ -4955,13 +4955,11 @@ void trace_profile(int argc, char **argv)
{
struct common_record_context ctx;

- init_common_record_context(&ctx, CMD_profile);
+ parse_record_options(argc, argv, CMD_profile, &ctx);

handle_init = trace_init_profile;
ctx.events = 1;

- parse_record_options(argc, argv, &ctx);
-
/*
* If no instances were set, then enable profiling on the top instance.
*/
@@ -4977,8 +4975,7 @@ void trace_record(int argc, char **argv)
{
struct common_record_context ctx;

- init_common_record_context(&ctx, CMD_record);
- parse_record_options(argc, argv, &ctx);
+ parse_record_options(argc, argv, CMD_record, &ctx);
record_trace(argc, argv, &ctx);
exit(0);
}
--
2.14.1


From 1585548351389879780@xxx Fri Dec 01 02:52:20 +0000 2017
X-GM-THRID: 1585548351389879780
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

Subject: [PATCH v2 08/10] trace-cmd: Introducing get_trace_cmd_type()

This patch aims to reduce the size of common_record_commads_code() by removing
a relatively long 'else if' sequence that was used to do the mapping between the
current trace command and the trace_type used by it.

Signed-off-by: Vladislav Valtchev (VMware) <[email protected]>
---
trace-record.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index 7688565..ec0eaed 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4746,6 +4746,27 @@ static void parse_record_options(int argc,
}
}

+static enum trace_type get_trace_cmd_type(enum trace_cmd cmd)
+{
+ const static struct {
+ enum trace_cmd cmd;
+ enum trace_type ttype;
+ } trace_type_per_command[] = {
+ {CMD_record, TRACE_TYPE_RECORD},
+ {CMD_stream, TRACE_TYPE_STREAM},
+ {CMD_extract, TRACE_TYPE_EXTRACT},
+ {CMD_profile, TRACE_TYPE_STREAM},
+ {CMD_start, TRACE_TYPE_START}
+ };
+
+ for (int i = 0; i < ARRAY_SIZE(trace_type_per_command); i++) {
+ if (trace_type_per_command[i].cmd == cmd)
+ return trace_type_per_command[i].ttype;
+ }
+
+ die("Trace type UNKNOWN for the given cmd_fun");
+}
+
/*
* This function contains common code for the following commands:
* record, start, extract, stream, profile.
@@ -4753,7 +4774,7 @@ static void parse_record_options(int argc,
static void record_trace(int argc, char **argv,
struct common_record_context *ctx)
{
- enum trace_type type = 0;
+ enum trace_type type = get_trace_cmd_type(ctx->curr_cmd);

/*
* If top_instance doesn't have any plugins or events, then
@@ -4820,17 +4841,6 @@ static void record_trace(int argc, char **argv,
set_buffer_size();
}

- if (IS_RECORD(ctx))
- type = TRACE_TYPE_RECORD;
- else if (IS_STREAM(ctx))
- type = TRACE_TYPE_STREAM;
- else if (IS_EXTRACT(ctx))
- type = TRACE_TYPE_EXTRACT;
- else if (IS_PROFILE(ctx))
- type = TRACE_TYPE_STREAM;
- else
- type = TRACE_TYPE_START;
-
update_plugins(type);

set_options();
--
2.14.1


From 1585503167020955150@xxx Thu Nov 30 14:54:08 +0000 2017
X-GM-THRID: 1585503167020955150
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

Subject: [PATCH v2 09/10] trace-cmd: Extract finalize_record_trace()

This patch splits record_trace() in two parts by moving its finalization part
in a separate function. This will also allow splitting out trace-cmd extract
code from trace-cmd record code, by using a shared function.

Signed-off-by: Vladislav Valtchev (VMware) <[email protected]>
---
trace-record.c | 48 ++++++++++++++++++++++++++----------------------
1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index ec0eaed..749c205 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4767,6 +4767,31 @@ static enum trace_type get_trace_cmd_type(enum trace_cmd cmd)
die("Trace type UNKNOWN for the given cmd_fun");
}

+static void finalize_record_trace(struct common_record_context *ctx)
+{
+ if (keep)
+ return;
+
+ update_reset_files();
+ update_reset_triggers();
+ if (clear_function_filters)
+ clear_func_filters();
+
+ set_plugin("nop");
+
+ tracecmd_remove_instances();
+
+ /* If tracing_on was enabled before we started, set it on now */
+ for_all_instances(ctx->instance) {
+ if (ctx->instance->keep)
+ write_tracing_on(ctx->instance,
+ ctx->instance->tracing_on_init_val);
+ }
+
+ if (host)
+ tracecmd_output_close(network_handle);
+}
+
/*
* This function contains common code for the following commands:
* record, start, extract, stream, profile.
@@ -4910,28 +4935,7 @@ static void record_trace(int argc, char **argv,
print_stats();

destroy_stats();
-
- if (keep)
- return;
-
- update_reset_files();
- update_reset_triggers();
- if (clear_function_filters)
- clear_func_filters();
-
- set_plugin("nop");
-
- tracecmd_remove_instances();
-
- /* If tracing_on was enabled before we started, set it on now */
- for_all_instances(ctx->instance) {
- if (ctx->instance->keep)
- write_tracing_on(ctx->instance,
- ctx->instance->tracing_on_init_val);
- }
-
- if (host)
- tracecmd_output_close(network_handle);
+ finalize_record_trace(ctx);
}

void trace_start(int argc, char **argv)
--
2.14.1


From 1586124682115320704@xxx Thu Dec 07 11:32:51 +0000 2017
X-GM-THRID: 1586124682115320704
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

Subject: [PATCH v2 10/10] trace-cmd: Fork record_trace() for the extract case

This patch inlines record_trace() into trace_extract() by removing the code not
related to extract, by replacing IS_EXTRACT(ctx) with true and then removing the
dead code, as well as the if statements when their condition always evalutates
to true. The opposite change [IS_EXTRACT(ctx) evaluated as false] has been
applyed to record_trace().

The purpose of doing that is to reduce the amount of branches in both the cases
(extract and everything else), making the code simpler to understand and follow
but at the price of having some copy-pasted code.

Signed-off-by: Vladislav Valtchev (VMware) <[email protected]>
---
trace-record.c | 172 ++++++++++++++++++++++++++++++++++-----------------------
1 file changed, 102 insertions(+), 70 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index 749c205..48426e5 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4794,7 +4794,7 @@ static void finalize_record_trace(struct common_record_context *ctx)

/*
* This function contains common code for the following commands:
- * record, start, extract, stream, profile.
+ * record, start, stream, profile.
*/
static void record_trace(int argc, char **argv,
struct common_record_context *ctx)
@@ -4805,15 +4805,13 @@ static void record_trace(int argc, char **argv,
* If top_instance doesn't have any plugins or events, then
* remove it from being processed.
*/
- if (!IS_EXTRACT(ctx) && !__check_doing_something(&top_instance))
+ if (!__check_doing_something(&top_instance))
first_instance = buffer_instances;
else
ctx->topt = 1;

update_first_instance(ctx->instance, ctx->topt);
-
- if (!IS_EXTRACT(ctx))
- check_doing_something();
+ check_doing_something();
check_function_plugin();

if (ctx->output)
@@ -4831,43 +4829,35 @@ static void record_trace(int argc, char **argv,
ctx->instance->tracing_on_init_val = 1;
}

- /* Extracting data records all events in the system. */
- if (IS_EXTRACT(ctx) && !ctx->record_all)
- record_all_events();
-
- if (!IS_EXTRACT(ctx))
- make_instances();
+ make_instances();

if (ctx->events)
expand_event_list();

page_size = getpagesize();

- if (!IS_EXTRACT(ctx)) {
- fset = set_ftrace(!ctx->disable, ctx->total_disable);
- tracecmd_disable_all_tracing(1);
+ fset = set_ftrace(!ctx->disable, ctx->total_disable);
+ tracecmd_disable_all_tracing(1);

+ for_all_instances(ctx->instance)
+ set_clock(ctx->instance);
+
+ /* Record records the date first */
+ if (IS_RECORD(ctx) && ctx->date)
+ ctx->date2ts = get_date_to_ts();
+
+ for_all_instances(ctx->instance) {
+ set_funcs(ctx->instance);
+ set_mask(ctx->instance);
+ }
+
+ if (ctx->events) {
for_all_instances(ctx->instance)
- set_clock(ctx->instance);
-
- /* Record records the date first */
- if (IS_RECORD(ctx) && ctx->date)
- ctx->date2ts = get_date_to_ts();
-
- for_all_instances(ctx->instance) {
- set_funcs(ctx->instance);
- set_mask(ctx->instance);
- }
-
- if (ctx->events) {
- for_all_instances(ctx->instance)
- enable_events(ctx->instance);
- }
- set_buffer_size();
+ enable_events(ctx->instance);
}

+ set_buffer_size();
update_plugins(type);
-
set_options();

if (ctx->max_graph_depth) {
@@ -4882,53 +4872,36 @@ static void record_trace(int argc, char **argv,
signal(SIGINT, finish);
if (!latency)
start_threads(type, ctx->global);
- }
-
- if (IS_EXTRACT(ctx)) {
- flush_threads();
-
} else {
- if (!(type & (TRACE_TYPE_RECORD | TRACE_TYPE_STREAM))) {
- update_task_filter();
- tracecmd_enable_tracing();
- exit(0);
- }
-
- if (ctx->run_command)
- run_cmd(type, (argc - optind) - 1, &argv[optind + 1]);
- else {
- update_task_filter();
- tracecmd_enable_tracing();
- /* We don't ptrace ourself */
- if (do_ptrace && filter_pid >= 0)
- ptrace_attach(filter_pid);
- /* sleep till we are woken with Ctrl^C */
- printf("Hit Ctrl^C to stop recording\n");
- while (!finished)
- trace_or_sleep(type);
- }
+ update_task_filter();
+ tracecmd_enable_tracing();
+ exit(0);
+ }

- tracecmd_disable_tracing();
- if (!latency)
- stop_threads(type);
+ if (ctx->run_command)
+ run_cmd(type, (argc - optind) - 1, &argv[optind + 1]);
+ else {
+ update_task_filter();
+ tracecmd_enable_tracing();
+ /* We don't ptrace ourself */
+ if (do_ptrace && filter_pid >= 0)
+ ptrace_attach(filter_pid);
+ /* sleep till we are woken with Ctrl^C */
+ printf("Hit Ctrl^C to stop recording\n");
+ while (!finished)
+ trace_or_sleep(type);
}

+ tracecmd_disable_tracing();
+ if (!latency)
+ stop_threads(type);
+
record_stats();

if (!keep)
tracecmd_disable_all_tracing(0);

- /* extract records the date after extraction */
- if (IS_EXTRACT(ctx) && ctx->date) {
- /*
- * We need to start tracing, don't let other traces
- * screw with our trace_marker.
- */
- tracecmd_disable_all_tracing(1);
- ctx->date2ts = get_date_to_ts();
- }
-
- if (IS_RECORD(ctx) || IS_EXTRACT(ctx)) {
+ if (IS_RECORD(ctx)) {
record_data(ctx->date2ts, ctx->data_flags);
delete_thread_data();
} else
@@ -4950,9 +4923,68 @@ void trace_start(int argc, char **argv)
void trace_extract(int argc, char **argv)
{
struct common_record_context ctx;
+ enum trace_type type;

parse_record_options(argc, argv, CMD_extract, &ctx);
- record_trace(argc, argv, &ctx);
+
+ type = get_trace_cmd_type(ctx.curr_cmd);
+
+ update_first_instance(ctx.instance, 1);
+ check_function_plugin();
+
+ if (ctx.output)
+ output_file = ctx.output;
+
+ /* Save the state of tracing_on before starting */
+ for_all_instances(ctx.instance) {
+
+ if (!ctx.manual && ctx.instance->profile)
+ enable_profile(ctx.instance);
+
+ ctx.instance->tracing_on_init_val = read_tracing_on(ctx.instance);
+ /* Some instances may not be created yet */
+ if (ctx.instance->tracing_on_init_val < 0)
+ ctx.instance->tracing_on_init_val = 1;
+ }
+
+ /* Extracting data records all events in the system. */
+ if (!ctx.record_all)
+ record_all_events();
+
+ if (ctx.events)
+ expand_event_list();
+
+ page_size = getpagesize();
+ update_plugins(type);
+ set_options();
+
+ if (ctx.max_graph_depth) {
+ for_all_instances(ctx.instance)
+ set_max_graph_depth(ctx.instance, ctx.max_graph_depth);
+ free(ctx.max_graph_depth);
+ }
+
+ allocate_seq();
+ flush_threads();
+ record_stats();
+
+ if (!keep)
+ tracecmd_disable_all_tracing(0);
+
+ /* extract records the date after extraction */
+ if (ctx.date) {
+ /*
+ * We need to start tracing, don't let other traces
+ * screw with our trace_marker.
+ */
+ tracecmd_disable_all_tracing(1);
+ ctx.date2ts = get_date_to_ts();
+ }
+
+ record_data(ctx.date2ts, ctx.data_flags);
+ delete_thread_data();
+ destroy_stats();
+ finalize_record_trace(&ctx);
exit(0);
}

--
2.14.1


From 1585465066680048844@xxx Thu Nov 30 04:48:33 +0000 2017
X-GM-THRID: 1582777349862922949
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

Subject: [PATCH v2 01/10] trace-cmd: Extract parse_record_options() from trace_record()

In this patch a huge part of trace_record(), dealing with parsing the command
line options, has been extracted in a separate function. That allows the body
of trace_record() to be focused only on the proper actions involved in a given
type of tracing (record, stream, etc.) reducing, this way, the scope and the
complexity of trace_record().

Signed-off-by: Vladislav Valtchev (VMware) <[email protected]>
---
trace-record.c | 335 ++++++++++++++++++++++++++++++---------------------------
1 file changed, 179 insertions(+), 156 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index 770b5bd..487d35e 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4358,62 +4358,57 @@ void trace_reset(int argc, char **argv)
exit(0);
}

-void trace_record(int argc, char **argv)
+struct common_record_context {
+ struct buffer_instance *instance;
+ const char *output;
+ char *date2ts;
+ char *max_graph_depth;
+ int data_flags;
+
+ int record_all;
+ int total_disable;
+ int disable;
+ int events;
+ int global;
+ int filtered;
+ int date;
+ int manual;
+ int topt;
+ int do_child;
+ int run_command;
+
+ int extract;
+ int start;
+ int stream;
+ int profile;
+ int record;
+};
+
+static void init_common_record_context(struct common_record_context *ctx)
+{
+ memset(ctx, 0, sizeof(*ctx));
+ ctx->instance = &top_instance;
+ init_instance(ctx->instance);
+ cpu_count = count_cpus();
+}
+
+static void parse_record_options(int argc,
+ char **argv,
+ struct common_record_context *ctx)
{
const char *plugin = NULL;
- const char *output = NULL;
const char *option;
struct event_list *event = NULL;
struct event_list *last_event = NULL;
- struct buffer_instance *instance = &top_instance;
- enum trace_type type = 0;
char *pids;
char *pid;
char *sav;
- char *date2ts = NULL;
- int record_all = 0;
- int total_disable = 0;
- int disable = 0;
- int events = 0;
- int record = 0;
- int extract = 0;
- int stream = 0;
- int profile = 0;
- int global = 0;
- int start = 0;
- int filtered = 0;
- int run_command = 0;
int neg_event = 0;
- int date = 0;
- int manual = 0;
- char *max_graph_depth = NULL;
- int topt = 0;
- int do_child = 0;
- int data_flags = 0;
-
- int c;
-
- init_instance(instance);
-
- cpu_count = count_cpus();
-
- if ((record = (strcmp(argv[1], "record") == 0)))
- ; /* do nothing */
- else if ((start = strcmp(argv[1], "start") == 0))
- ; /* do nothing */
- else if ((extract = strcmp(argv[1], "extract") == 0))
- ; /* do nothing */
- else if ((stream = strcmp(argv[1], "stream") == 0))
- ; /* do nothing */
- else if ((profile = strcmp(argv[1], "profile") == 0)) {
- handle_init = trace_init_profile;
- events = 1;
- } else
- usage(argv);

for (;;) {
int option_index = 0;
int ret;
+ int c;
const char *opts;
static struct option long_options[] = {
{"date", no_argument, NULL, OPT_date},
@@ -4431,7 +4426,7 @@ void trace_record(int argc, char **argv)
{NULL, 0, NULL, 0}
};

- if (extract)
+ if (ctx->extract)
opts = "+haf:Fp:co:O:sr:g:l:n:P:N:tb:B:ksiT";
else
opts = "+hae:f:Fp:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
@@ -4443,26 +4438,26 @@ void trace_record(int argc, char **argv)
usage(argv);
break;
case 'a':
- if (extract) {
+ if (ctx->extract) {
add_all_instances();
} else {
- record_all = 1;
+ ctx->record_all = 1;
record_all_events();
}
break;
case 'e':
- events = 1;
+ ctx->events = 1;
event = malloc(sizeof(*event));
if (!event)
die("Failed to allocate event %s", optarg);
memset(event, 0, sizeof(*event));
event->event = optarg;
- add_event(instance, event);
+ add_event(ctx->instance, event);
event->neg = neg_event;
event->filter = NULL;
last_event = event;

- if (!record_all)
+ if (!ctx->record_all)
list_event(optarg);
break;
case 'f':
@@ -4495,7 +4490,7 @@ void trace_record(int argc, char **argv)
filter_task = 1;
break;
case 'G':
- global = 1;
+ ctx->global = 1;
break;
case 'P':
test_set_event_pid();
@@ -4518,35 +4513,35 @@ void trace_record(int argc, char **argv)
do_ptrace = 1;
} else {
save_option("event-fork");
- do_child = 1;
+ ctx->do_child = 1;
}
break;
case 'C':
- instance->clock = optarg;
+ ctx->instance->clock = optarg;
break;
case 'v':
neg_event = 1;
break;
case 'l':
- add_func(&instance->filter_funcs,
- instance->filter_mod, optarg);
- filtered = 1;
+ add_func(&ctx->instance->filter_funcs,
+ ctx->instance->filter_mod, optarg);
+ ctx->filtered = 1;
break;
case 'n':
- add_func(&instance->notrace_funcs,
- instance->filter_mod, optarg);
- filtered = 1;
+ add_func(&ctx->instance->notrace_funcs,
+ ctx->instance->filter_mod, optarg);
+ ctx->filtered = 1;
break;
case 'g':
- add_func(&graph_funcs, instance->filter_mod, optarg);
- filtered = 1;
+ add_func(&graph_funcs, ctx->instance->filter_mod, optarg);
+ ctx->filtered = 1;
break;
case 'p':
- if (instance->plugin)
+ if (ctx->instance->plugin)
die("only one plugin allowed");
for (plugin = optarg; isspace(*plugin); plugin++)
;
- instance->plugin = plugin;
+ ctx->instance->plugin = plugin;
for (optarg += strlen(optarg) - 1;
optarg > plugin && isspace(*optarg); optarg--)
;
@@ -4554,25 +4549,25 @@ void trace_record(int argc, char **argv)
optarg[0] = '\0';
break;
case 'D':
- total_disable = 1;
+ ctx->total_disable = 1;
/* fall through */
case 'd':
- disable = 1;
+ ctx->disable = 1;
break;
case 'o':
if (host)
die("-o incompatible with -N");
- if (start)
+ if (ctx->start)
die("start does not take output\n"
"Did you mean 'record'?");
- if (stream)
+ if (ctx->stream)
die("stream does not take output\n"
"Did you mean 'record'?");
- if (output)
+ if (ctx->output)
die("only one output file allowed");
- output = optarg;
+ ctx->output = optarg;

- if (profile) {
+ if (ctx->profile) {
int fd;

/* pipe the output to this file instead of stdout */
@@ -4595,11 +4590,11 @@ void trace_record(int argc, char **argv)
save_option("stacktrace");
break;
case 'H':
- add_hook(instance, optarg);
- events = 1;
+ add_hook(ctx->instance, optarg);
+ ctx->events = 1;
break;
case 's':
- if (extract) {
+ if (ctx->extract) {
if (optarg)
usage(argv);
recorder_flags |= TRACECMD_RECORD_SNAPSHOT;
@@ -4610,47 +4605,47 @@ void trace_record(int argc, char **argv)
sleep_time = atoi(optarg);
break;
case 'S':
- manual = 1;
+ ctx->manual = 1;
/* User sets events for profiling */
if (!event)
- events = 0;
+ ctx->events = 0;
break;
case 'r':
rt_prio = atoi(optarg);
break;
case 'N':
- if (!record)
+ if (!ctx->record)
die("-N only available with record");
- if (output)
+ if (ctx->output)
die("-N incompatible with -o");
host = optarg;
break;
case 'm':
if (max_kb)
die("-m can only be specified once");
- if (!record)
+ if (!ctx->record)
die("only record take 'm' option");
max_kb = atoi(optarg);
break;
case 'M':
- instance->cpumask = alloc_mask_from_hex(optarg);
+ ctx->instance->cpumask = alloc_mask_from_hex(optarg);
break;
case 't':
- if (extract)
- topt = 1; /* Extract top instance also */
+ if (ctx->extract)
+ ctx->topt = 1; /* Extract top instance also */
else
use_tcp = 1;
break;
case 'b':
- instance->buffer_size = atoi(optarg);
+ ctx->instance->buffer_size = atoi(optarg);
break;
case 'B':
- instance = create_instance(optarg);
- if (!instance)
+ ctx->instance = create_instance(optarg);
+ if (!ctx->instance)
die("Failed to create instance");
- add_instance(instance);
- if (profile)
- instance->profile = 1;
+ add_instance(ctx->instance);
+ if (ctx->profile)
+ ctx->instance->profile = 1;
break;
case 'k':
keep = 1;
@@ -4659,10 +4654,10 @@ void trace_record(int argc, char **argv)
ignore_event_not_found = 1;
break;
case OPT_date:
- date = 1;
- if (data_flags & DATA_FL_OFFSET)
+ ctx->date = 1;
+ if (ctx->data_flags & DATA_FL_OFFSET)
die("Can not use both --date and --ts-offset");
- data_flags |= DATA_FL_DATE;
+ ctx->data_flags |= DATA_FL_DATE;
break;
case OPT_funcstack:
func_stack = 1;
@@ -4672,8 +4667,8 @@ void trace_record(int argc, char **argv)
break;
case OPT_profile:
handle_init = trace_init_profile;
- instance->profile = 1;
- events = 1;
+ ctx->instance->profile = 1;
+ ctx->events = 1;
break;
case OPT_stderr:
/* if -o was used (for profile), ignore this */
@@ -4687,26 +4682,26 @@ void trace_record(int argc, char **argv)
trace_profile_set_merge_like_comms();
break;
case OPT_tsoffset:
- date2ts = strdup(optarg);
- if (data_flags & DATA_FL_DATE)
+ ctx->date2ts = strdup(optarg);
+ if (ctx->data_flags & DATA_FL_DATE)
die("Can not use both --date and --ts-offset");
- data_flags |= DATA_FL_OFFSET;
+ ctx->data_flags |= DATA_FL_OFFSET;
break;
case OPT_max_graph_depth:
- free(max_graph_depth);
- max_graph_depth = strdup(optarg);
- if (!max_graph_depth)
+ free(ctx->max_graph_depth);
+ ctx->max_graph_depth = strdup(optarg);
+ if (!ctx->max_graph_depth)
die("Could not allocate option");
break;
case OPT_debug:
debug = 1;
break;
case OPT_module:
- if (instance->filter_mod)
- add_func(&instance->filter_funcs,
- instance->filter_mod, "*");
- instance->filter_mod = optarg;
- filtered = 0;
+ if (ctx->instance->filter_mod)
+ add_func(&ctx->instance->filter_funcs,
+ ctx->instance->filter_mod, "*");
+ ctx->instance->filter_mod = optarg;
+ ctx->filtered = 0;
break;
case OPT_quiet:
case 'q':
@@ -4717,104 +4712,131 @@ void trace_record(int argc, char **argv)
}
}

- if (!filtered && instance->filter_mod)
- add_func(&instance->filter_funcs,
- instance->filter_mod, "*");
+ if (!ctx->filtered && ctx->instance->filter_mod)
+ add_func(&ctx->instance->filter_funcs,
+ ctx->instance->filter_mod, "*");

if (do_ptrace && !filter_task && (filter_pid < 0))
die(" -c can only be used with -F (or -P with event-fork support)");
- if (do_child && !filter_task &&! filter_pid)
+ if (ctx->do_child && !filter_task &&! filter_pid)
die(" -c can only be used with -P or -F");

if ((argc - optind) >= 2) {
- if (start)
+ if (ctx->start)
die("Command start does not take any commands\n"
"Did you mean 'record'?");
- if (extract)
+ if (ctx->extract)
die("Command extract does not take any commands\n"
"Did you mean 'record'?");
- run_command = 1;
+ ctx->run_command = 1;
}

+}
+
+void trace_record(int argc, char **argv)
+{
+ struct common_record_context ctx;
+ enum trace_type type = 0;
+
+ init_common_record_context(&ctx);
+
+ if ((ctx.record = (strcmp(argv[1], "record") == 0)))
+ ; /* do nothing */
+ else if ((ctx.start = strcmp(argv[1], "start") == 0))
+ ; /* do nothing */
+ else if ((ctx.extract = strcmp(argv[1], "extract") == 0))
+ ; /* do nothing */
+ else if ((ctx.stream = strcmp(argv[1], "stream") == 0))
+ ; /* do nothing */
+ else if ((ctx.profile = strcmp(argv[1], "profile") == 0)) {
+ handle_init = trace_init_profile;
+ ctx.events = 1;
+ } else
+ usage(argv);
+
+
+ parse_record_options(argc, argv, &ctx);
+
+
/*
* If this is a profile run, and no instances were set,
* then enable profiling on the top instance.
*/
- if (profile && !buffer_instances)
+ if (ctx.profile && !buffer_instances)
top_instance.profile = 1;

/*
* If top_instance doesn't have any plugins or events, then
* remove it from being processed.
*/
- if (!extract && !__check_doing_something(&top_instance))
+ if (!ctx.extract && !__check_doing_something(&top_instance))
first_instance = buffer_instances;
else
- topt = 1;
+ ctx.topt = 1;

- update_first_instance(instance, topt);
+ update_first_instance(ctx.instance, ctx.topt);

- if (!extract)
+ if (!ctx.extract)
check_doing_something();
check_function_plugin();

- if (output)
- output_file = output;
+ if (ctx.output)
+ output_file = ctx.output;

/* Save the state of tracing_on before starting */
- for_all_instances(instance) {
+ for_all_instances(ctx.instance) {

- if (!manual && instance->profile)
- enable_profile(instance);
+ if (!ctx.manual && ctx.instance->profile)
+ enable_profile(ctx.instance);

- instance->tracing_on_init_val = read_tracing_on(instance);
+ ctx.instance->tracing_on_init_val = read_tracing_on(ctx.instance);
/* Some instances may not be created yet */
- if (instance->tracing_on_init_val < 0)
- instance->tracing_on_init_val = 1;
+ if (ctx.instance->tracing_on_init_val < 0)
+ ctx.instance->tracing_on_init_val = 1;
}

/* Extracting data records all events in the system. */
- if (extract && !record_all)
+ if (ctx.extract && !ctx.record_all)
record_all_events();

- if (!extract)
+ if (!ctx.extract)
make_instances();

- if (events)
+ if (ctx.events)
expand_event_list();

page_size = getpagesize();

- if (!extract) {
- fset = set_ftrace(!disable, total_disable);
+ if (!ctx.extract) {
+ fset = set_ftrace(!ctx.disable, ctx.total_disable);
tracecmd_disable_all_tracing(1);

- for_all_instances(instance)
- set_clock(instance);
+ for_all_instances(ctx.instance)
+ set_clock(ctx.instance);

/* Record records the date first */
- if (record && date)
- date2ts = get_date_to_ts();
+ if (ctx.record && ctx.date)
+ ctx.date2ts = get_date_to_ts();

- for_all_instances(instance) {
- set_funcs(instance);
- set_mask(instance);
+ for_all_instances(ctx.instance) {
+ set_funcs(ctx.instance);
+ set_mask(ctx.instance);
}

- if (events) {
- for_all_instances(instance)
- enable_events(instance);
+ if (ctx.events) {
+ for_all_instances(ctx.instance)
+ enable_events(ctx.instance);
}
set_buffer_size();
}

- if (record)
+ if (ctx.record)
type = TRACE_TYPE_RECORD;
- else if (stream)
+ else if (ctx.stream)
type = TRACE_TYPE_STREAM;
- else if (extract)
+ else if (ctx.extract)
type = TRACE_TYPE_EXTRACT;
- else if (profile)
+ else if (ctx.profile)
type = TRACE_TYPE_STREAM;
else
type = TRACE_TYPE_START;
@@ -4823,10 +4845,10 @@ void trace_record(int argc, char **argv)

set_options();

- if (max_graph_depth) {
- for_all_instances(instance)
- set_max_graph_depth(instance, max_graph_depth);
- free(max_graph_depth);
+ if (ctx.max_graph_depth) {
+ for_all_instances(ctx.instance)
+ set_max_graph_depth(ctx.instance, ctx.max_graph_depth);
+ free(ctx.max_graph_depth);
}

allocate_seq();
@@ -4834,10 +4856,10 @@ void trace_record(int argc, char **argv)
if (type & (TRACE_TYPE_RECORD | TRACE_TYPE_STREAM)) {
signal(SIGINT, finish);
if (!latency)
- start_threads(type, global);
+ start_threads(type, ctx.global);
}

- if (extract) {
+ if (ctx.extract) {
flush_threads();

} else {
@@ -4847,7 +4869,7 @@ void trace_record(int argc, char **argv)
exit(0);
}

- if (run_command)
+ if (ctx.run_command)
run_cmd(type, (argc - optind) - 1, &argv[optind + 1]);
else {
update_task_filter();
@@ -4872,17 +4894,17 @@ void trace_record(int argc, char **argv)
tracecmd_disable_all_tracing(0);

/* extract records the date after extraction */
- if (extract && date) {
+ if (ctx.extract && ctx.date) {
/*
* We need to start tracing, don't let other traces
* screw with our trace_marker.
*/
tracecmd_disable_all_tracing(1);
- date2ts = get_date_to_ts();
+ ctx.date2ts = get_date_to_ts();
}

- if (record || extract) {
- record_data(date2ts, data_flags);
+ if (ctx.record || ctx.extract) {
+ record_data(ctx.date2ts, ctx.data_flags);
delete_thread_data();
} else
print_stats();
@@ -4902,15 +4924,16 @@ void trace_record(int argc, char **argv)
tracecmd_remove_instances();

/* If tracing_on was enabled before we started, set it on now */
- for_all_instances(instance) {
- if (instance->keep)
- write_tracing_on(instance, instance->tracing_on_init_val);
+ for_all_instances(ctx.instance) {
+ if (ctx.instance->keep)
+ write_tracing_on(ctx.instance,
+ ctx.instance->tracing_on_init_val);
}

if (host)
tracecmd_output_close(network_handle);

- if (profile)
+ if (ctx.profile)
trace_profile();

exit(0);
--
2.14.1


From 1586616725005702369@xxx Tue Dec 12 21:53:40 +0000 2017
X-GM-THRID: 1586616725005702369
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

Subject: [PATCH v2 04/10] trace-cmd: Rename trace_profile() to do_trace_profile()

The purpose of this renaming is to make room for a new trace_profile() function,
that will handle directly the 'profile' command, following the internal
convention COMMAND_NAME -> trace_{COMMAND_NAME}.

Clearly, in the next steps the top-level trace_profile() function will be made
to call do_trace_profile() after some initialization.

Signed-off-by: Vladislav Valtchev (VMware) <[email protected]>
---
trace-local.h | 2 +-
trace-profile.c | 2 +-
trace-read.c | 2 +-
trace-record.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/trace-local.h b/trace-local.h
index c01ef72..fa5232b 100644
--- a/trace-local.h
+++ b/trace-local.h
@@ -96,7 +96,7 @@ struct hook_list;

void trace_init_profile(struct tracecmd_input *handle, struct hook_list *hooks,
int global);
-int trace_profile(void);
+int do_trace_profile(void);
void trace_profile_set_merge_like_comms(void);

struct tracecmd_input *
diff --git a/trace-profile.c b/trace-profile.c
index 0af27cd..a2bb3fc 100644
--- a/trace-profile.c
+++ b/trace-profile.c
@@ -2452,7 +2452,7 @@ static void merge_tasks(struct handle_data *h)
}
}

-int trace_profile(void)
+int do_trace_profile(void)
{
struct handle_data *h;

diff --git a/trace-read.c b/trace-read.c
index 350a843..af5548a 100644
--- a/trace-read.c
+++ b/trace-read.c
@@ -1227,7 +1227,7 @@ static void read_data_info(struct list_head *handle_list, enum output_type otype
} while (last_record);

if (profile)
- trace_profile();
+ do_trace_profile();

list_for_each_entry(handles, handle_list, list) {
free_filters(handles->event_filters);
diff --git a/trace-record.c b/trace-record.c
index b37e073..827189d 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4923,7 +4923,7 @@ static void record_trace(int argc, char **argv,
tracecmd_output_close(network_handle);

if (IS_PROFILE(ctx))
- trace_profile();
+ do_trace_profile();

exit(0);
}
--
2.14.1


From 1585441463890172484@xxx Wed Nov 29 22:33:24 +0000 2017
X-GM-THRID: 1585441463890172484
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

Subject: [PATCH v2 02/10] trace-cmd: Replacing cmd flags w/ a trace_cmd enum

This patch replaces 5 mutually exclusive flag variables (extract, start, stream,
record, profile) in trace_record() with a more convenient enum. The point of
doing that is to make the code simpler and easier to maintain.

Signed-off-by: Vladislav Valtchev (VMware) <[email protected]>
---
trace-record.c | 94 ++++++++++++++++++++++++++++++++--------------------------
1 file changed, 52 insertions(+), 42 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index 487d35e..9e35de4 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4358,7 +4358,16 @@ void trace_reset(int argc, char **argv)
exit(0);
}

+enum trace_cmd {
+ CMD_extract,
+ CMD_start,
+ CMD_stream,
+ CMD_profile,
+ CMD_record
+};
+
struct common_record_context {
+ enum trace_cmd curr_cmd;
struct buffer_instance *instance;
const char *output;
char *date2ts;
@@ -4376,12 +4385,6 @@ struct common_record_context {
int topt;
int do_child;
int run_command;
-
- int extract;
- int start;
- int stream;
- int profile;
- int record;
};

static void init_common_record_context(struct common_record_context *ctx)
@@ -4392,6 +4395,12 @@ static void init_common_record_context(struct common_record_context *ctx)
cpu_count = count_cpus();
}

+#define IS_EXTRACT(ctx) ((ctx)->curr_cmd == CMD_extract)
+#define IS_START(ctx) ((ctx)->curr_cmd == CMD_start)
+#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)
+
static void parse_record_options(int argc,
char **argv,
struct common_record_context *ctx)
@@ -4426,7 +4435,7 @@ static void parse_record_options(int argc,
{NULL, 0, NULL, 0}
};

- if (ctx->extract)
+ if (IS_EXTRACT(ctx))
opts = "+haf:Fp:co:O:sr:g:l:n:P:N:tb:B:ksiT";
else
opts = "+hae:f:Fp:cC:dDGo:O:s:r:vg:l:n:P:N:tb:R:B:ksSiTm:M:H:q";
@@ -4438,7 +4447,7 @@ static void parse_record_options(int argc,
usage(argv);
break;
case 'a':
- if (ctx->extract) {
+ if (IS_EXTRACT(ctx)) {
add_all_instances();
} else {
ctx->record_all = 1;
@@ -4557,17 +4566,17 @@ static void parse_record_options(int argc,
case 'o':
if (host)
die("-o incompatible with -N");
- if (ctx->start)
+ if (IS_START(ctx))
die("start does not take output\n"
"Did you mean 'record'?");
- if (ctx->stream)
+ if (IS_STREAM(ctx))
die("stream does not take output\n"
"Did you mean 'record'?");
if (ctx->output)
die("only one output file allowed");
ctx->output = optarg;

- if (ctx->profile) {
+ if (IS_PROFILE(ctx)) {
int fd;

/* pipe the output to this file instead of stdout */
@@ -4594,7 +4603,7 @@ static void parse_record_options(int argc,
ctx->events = 1;
break;
case 's':
- if (ctx->extract) {
+ if (IS_EXTRACT(ctx)) {
if (optarg)
usage(argv);
recorder_flags |= TRACECMD_RECORD_SNAPSHOT;
@@ -4614,7 +4623,7 @@ static void parse_record_options(int argc,
rt_prio = atoi(optarg);
break;
case 'N':
- if (!ctx->record)
+ if (!IS_RECORD(ctx))
die("-N only available with record");
if (ctx->output)
die("-N incompatible with -o");
@@ -4623,7 +4632,7 @@ static void parse_record_options(int argc,
case 'm':
if (max_kb)
die("-m can only be specified once");
- if (!ctx->record)
+ if (!IS_RECORD(ctx))
die("only record take 'm' option");
max_kb = atoi(optarg);
break;
@@ -4631,7 +4640,7 @@ static void parse_record_options(int argc,
ctx->instance->cpumask = alloc_mask_from_hex(optarg);
break;
case 't':
- if (ctx->extract)
+ if (IS_EXTRACT(ctx))
ctx->topt = 1; /* Extract top instance also */
else
use_tcp = 1;
@@ -4644,7 +4653,7 @@ static void parse_record_options(int argc,
if (!ctx->instance)
die("Failed to create instance");
add_instance(ctx->instance);
- if (ctx->profile)
+ if (IS_PROFILE(ctx))
ctx->instance->profile = 1;
break;
case 'k':
@@ -4722,10 +4731,10 @@ static void parse_record_options(int argc,
die(" -c can only be used with -P or -F");

if ((argc - optind) >= 2) {
- if (ctx->start)
+ if (IS_START(ctx))
die("Command start does not take any commands\n"
"Did you mean 'record'?");
- if (ctx->extract)
+ if (IS_EXTRACT(ctx))
die("Command extract does not take any commands\n"
"Did you mean 'record'?");
ctx->run_command = 1;
@@ -4740,15 +4749,16 @@ void trace_record(int argc, char **argv)

init_common_record_context(&ctx);

- if ((ctx.record = (strcmp(argv[1], "record") == 0)))
- ; /* do nothing */
- else if ((ctx.start = strcmp(argv[1], "start") == 0))
- ; /* do nothing */
- else if ((ctx.extract = strcmp(argv[1], "extract") == 0))
- ; /* do nothing */
- else if ((ctx.stream = strcmp(argv[1], "stream") == 0))
- ; /* do nothing */
- else if ((ctx.profile = strcmp(argv[1], "profile") == 0)) {
+ if (strcmp(argv[1], "record") == 0)
+ ctx.curr_cmd = CMD_record;
+ else if (strcmp(argv[1], "start") == 0)
+ ctx.curr_cmd = CMD_start;
+ else if (strcmp(argv[1], "extract") == 0)
+ ctx.curr_cmd = CMD_extract;
+ else if (strcmp(argv[1], "stream") == 0)
+ ctx.curr_cmd = CMD_stream;
+ else if (strcmp(argv[1], "profile") == 0) {
+ ctx.curr_cmd = CMD_profile;
handle_init = trace_init_profile;
ctx.events = 1;
} else
@@ -4762,21 +4772,21 @@ void trace_record(int argc, char **argv)
* If this is a profile run, and no instances were set,
* then enable profiling on the top instance.
*/
- if (ctx.profile && !buffer_instances)
+ if (IS_PROFILE(&ctx) && !buffer_instances)
top_instance.profile = 1;

/*
* If top_instance doesn't have any plugins or events, then
* remove it from being processed.
*/
- if (!ctx.extract && !__check_doing_something(&top_instance))
+ if (!IS_EXTRACT(&ctx) && !__check_doing_something(&top_instance))
first_instance = buffer_instances;
else
ctx.topt = 1;

update_first_instance(ctx.instance, ctx.topt);

- if (!ctx.extract)
+ if (!IS_EXTRACT(&ctx))
check_doing_something();
check_function_plugin();

@@ -4796,10 +4806,10 @@ void trace_record(int argc, char **argv)
}

/* Extracting data records all events in the system. */
- if (ctx.extract && !ctx.record_all)
+ if (IS_EXTRACT(&ctx) && !ctx.record_all)
record_all_events();

- if (!ctx.extract)
+ if (!IS_EXTRACT(&ctx))
make_instances();

if (ctx.events)
@@ -4807,7 +4817,7 @@ void trace_record(int argc, char **argv)

page_size = getpagesize();

- if (!ctx.extract) {
+ if (!IS_EXTRACT(&ctx)) {
fset = set_ftrace(!ctx.disable, ctx.total_disable);
tracecmd_disable_all_tracing(1);

@@ -4815,7 +4825,7 @@ void trace_record(int argc, char **argv)
set_clock(ctx.instance);

/* Record records the date first */
- if (ctx.record && ctx.date)
+ if (IS_RECORD(&ctx) && ctx.date)
ctx.date2ts = get_date_to_ts();

for_all_instances(ctx.instance) {
@@ -4830,13 +4840,13 @@ void trace_record(int argc, char **argv)
set_buffer_size();
}

- if (ctx.record)
+ if (IS_RECORD(&ctx))
type = TRACE_TYPE_RECORD;
- else if (ctx.stream)
+ else if (IS_STREAM(&ctx))
type = TRACE_TYPE_STREAM;
- else if (ctx.extract)
+ else if (IS_EXTRACT(&ctx))
type = TRACE_TYPE_EXTRACT;
- else if (ctx.profile)
+ else if (IS_PROFILE(&ctx))
type = TRACE_TYPE_STREAM;
else
type = TRACE_TYPE_START;
@@ -4859,7 +4869,7 @@ void trace_record(int argc, char **argv)
start_threads(type, ctx.global);
}

- if (ctx.extract) {
+ if (IS_EXTRACT(&ctx)) {
flush_threads();

} else {
@@ -4894,7 +4904,7 @@ void trace_record(int argc, char **argv)
tracecmd_disable_all_tracing(0);

/* extract records the date after extraction */
- if (ctx.extract && ctx.date) {
+ if (IS_EXTRACT(&ctx) && ctx.date) {
/*
* We need to start tracing, don't let other traces
* screw with our trace_marker.
@@ -4903,7 +4913,7 @@ void trace_record(int argc, char **argv)
ctx.date2ts = get_date_to_ts();
}

- if (ctx.record || ctx.extract) {
+ if (IS_RECORD(&ctx) || IS_EXTRACT(&ctx)) {
record_data(ctx.date2ts, ctx.data_flags);
delete_thread_data();
} else
@@ -4933,7 +4943,7 @@ void trace_record(int argc, char **argv)
if (host)
tracecmd_output_close(network_handle);

- if (ctx.profile)
+ if (IS_PROFILE(&ctx))
trace_profile();

exit(0);
--
2.14.1


From 1585436587863567649@xxx Wed Nov 29 21:15:54 +0000 2017
X-GM-THRID: 1584454378809684710
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

Subject: [PATCH v2 05/10] trace-cmd: Making start,extract,stream,profile separate funcs

This simple patch make the above-mentioned commands independent from 'record'.
The point of doing so is to follow the convention of one entry-point per command
that is followed by most of trace-cmd's code. Ultimately that aims to prevent
the complexity to concentrate in a single point, making the code simpler to
maintain.

Signed-off-by: Vladislav Valtchev (VMware) <[email protected]>
---
trace-cmd.c | 8 +++----
trace-local.h | 8 +++++++
trace-record.c | 72 ++++++++++++++++++++++++++++++++++++++++++----------------
3 files changed, 64 insertions(+), 24 deletions(-)

diff --git a/trace-cmd.c b/trace-cmd.c
index dd1c108..6c2efa3 100644
--- a/trace-cmd.c
+++ b/trace-cmd.c
@@ -102,11 +102,11 @@ struct command commands[] = {
{"stack", trace_stack},
{"check-events", trace_check_events},
{"record", trace_record},
- {"start", trace_record},
- {"extract", trace_record},
+ {"start", trace_start},
+ {"extract", trace_extract},
{"stop", trace_stop},
- {"stream", trace_record},
- {"profile", trace_record},
+ {"stream", trace_stream},
+ {"profile", trace_profile},
{"restart", trace_restart},
{"reset", trace_reset},
{"stat", trace_stat},
diff --git a/trace-local.h b/trace-local.h
index fa5232b..eaae430 100644
--- a/trace-local.h
+++ b/trace-local.h
@@ -64,6 +64,14 @@ void trace_restart(int argc, char **argv);

void trace_reset(int argc, char **argv);

+void trace_start(int argc, char **argv);
+
+void trace_extract(int argc, char **argv);
+
+void trace_stream(int argc, char **argv);
+
+void trace_profile(int argc, char **argv);
+
void trace_report(int argc, char **argv);

void trace_split(int argc, char **argv);
diff --git a/trace-record.c b/trace-record.c
index 827189d..6c12416 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4387,10 +4387,12 @@ struct common_record_context {
int run_command;
};

-static void init_common_record_context(struct common_record_context *ctx)
+static void init_common_record_context(struct common_record_context *ctx,
+ enum trace_cmd curr_cmd)
{
memset(ctx, 0, sizeof(*ctx));
ctx->instance = &top_instance;
+ ctx->curr_cmd = curr_cmd;
init_instance(ctx->instance);
cpu_count = count_cpus();
}
@@ -4739,9 +4741,12 @@ static void parse_record_options(int argc,
"Did you mean 'record'?");
ctx->run_command = 1;
}
-
}

+/*
+ * This function contains common code for the following commands:
+ * record, start, extract, stream, profile.
+ */
static void record_trace(int argc, char **argv,
struct common_record_context *ctx)
{
@@ -4901,7 +4906,7 @@ static void record_trace(int argc, char **argv,
destroy_stats();

if (keep)
- exit(0);
+ return;

update_reset_files();
update_reset_triggers();
@@ -4924,7 +4929,49 @@ static void record_trace(int argc, char **argv,

if (IS_PROFILE(ctx))
do_trace_profile();
+}

+void trace_start(int argc, char **argv)
+{
+ struct common_record_context ctx;
+
+ init_common_record_context(&ctx, CMD_start);
+ parse_record_options(argc, argv, &ctx);
+ record_trace(argc, argv, &ctx);
+ exit(0);
+}
+
+void trace_extract(int argc, char **argv)
+{
+ struct common_record_context ctx;
+
+ init_common_record_context(&ctx, CMD_extract);
+ parse_record_options(argc, argv, &ctx);
+ record_trace(argc, argv, &ctx);
+ exit(0);
+}
+
+void trace_stream(int argc, char **argv)
+{
+ struct common_record_context ctx;
+
+ init_common_record_context(&ctx, CMD_stream);
+ parse_record_options(argc, argv, &ctx);
+ record_trace(argc, argv, &ctx);
+ exit(0);
+}
+
+void trace_profile(int argc, char **argv)
+{
+ struct common_record_context ctx;
+
+ init_common_record_context(&ctx, CMD_profile);
+
+ handle_init = trace_init_profile;
+ ctx.events = 1;
+
+ parse_record_options(argc, argv, &ctx);
+ record_trace(argc, argv, &ctx);
exit(0);
}

@@ -4932,23 +4979,8 @@ void trace_record(int argc, char **argv)
{
struct common_record_context ctx;

- init_common_record_context(&ctx);
-
- if (strcmp(argv[1], "record") == 0)
- ctx.curr_cmd = CMD_record;
- else if (strcmp(argv[1], "start") == 0)
- ctx.curr_cmd = CMD_start;
- else if (strcmp(argv[1], "extract") == 0)
- ctx.curr_cmd = CMD_extract;
- else if (strcmp(argv[1], "stream") == 0)
- ctx.curr_cmd = CMD_stream;
- else if (strcmp(argv[1], "profile") == 0) {
- ctx.curr_cmd = CMD_profile;
- handle_init = trace_init_profile;
- ctx.events = 1;
- } else
- usage(argv);
-
+ init_common_record_context(&ctx, CMD_record);
parse_record_options(argc, argv, &ctx);
record_trace(argc, argv, &ctx);
+ exit(0);
}
--
2.14.1


From 1585425621543637305@xxx Wed Nov 29 18:21:35 +0000 2017
X-GM-THRID: 1585425621543637305
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

Subject: [PATCH v2 03/10] trace-cmd: Extracting record_trace()

This patch moves in a separate function almost all the code in trace_record()
after the call of parse_record_options(). This is the last necessary preparation
step before removing the command-multiplexing code from trace_record and
allowing the commands 'start', 'extract', 'stream' and 'profile' to have an
independent entry-point from 'record'.

Signed-off-by: Vladislav Valtchev (VMware) <[email protected]>
---
trace-record.c | 146 +++++++++++++++++++++++++++++----------------------------
1 file changed, 75 insertions(+), 71 deletions(-)

diff --git a/trace-record.c b/trace-record.c
index 9e35de4..b37e073 100644
--- a/trace-record.c
+++ b/trace-record.c
@@ -4742,111 +4742,90 @@ static void parse_record_options(int argc,

}

-void trace_record(int argc, char **argv)
+static void record_trace(int argc, char **argv,
+ struct common_record_context *ctx)
{
- struct common_record_context ctx;
enum trace_type type = 0;

- init_common_record_context(&ctx);
-
- if (strcmp(argv[1], "record") == 0)
- ctx.curr_cmd = CMD_record;
- else if (strcmp(argv[1], "start") == 0)
- ctx.curr_cmd = CMD_start;
- else if (strcmp(argv[1], "extract") == 0)
- ctx.curr_cmd = CMD_extract;
- else if (strcmp(argv[1], "stream") == 0)
- ctx.curr_cmd = CMD_stream;
- else if (strcmp(argv[1], "profile") == 0) {
- ctx.curr_cmd = CMD_profile;
- handle_init = trace_init_profile;
- ctx.events = 1;
- } else
- usage(argv);
-
-
- parse_record_options(argc, argv, &ctx);
-
-
/*
* If this is a profile run, and no instances were set,
* then enable profiling on the top instance.
*/
- if (IS_PROFILE(&ctx) && !buffer_instances)
+ if (IS_PROFILE(ctx) && !buffer_instances)
top_instance.profile = 1;

/*
* If top_instance doesn't have any plugins or events, then
* remove it from being processed.
*/
- if (!IS_EXTRACT(&ctx) && !__check_doing_something(&top_instance))
+ if (!IS_EXTRACT(ctx) && !__check_doing_something(&top_instance))
first_instance = buffer_instances;
else
- ctx.topt = 1;
+ ctx->topt = 1;

- update_first_instance(ctx.instance, ctx.topt);
+ update_first_instance(ctx->instance, ctx->topt);

- if (!IS_EXTRACT(&ctx))
+ if (!IS_EXTRACT(ctx))
check_doing_something();
check_function_plugin();

- if (ctx.output)
- output_file = ctx.output;
+ if (ctx->output)
+ output_file = ctx->output;

/* Save the state of tracing_on before starting */
- for_all_instances(ctx.instance) {
+ for_all_instances(ctx->instance) {

- if (!ctx.manual && ctx.instance->profile)
- enable_profile(ctx.instance);
+ if (!ctx->manual && ctx->instance->profile)
+ enable_profile(ctx->instance);

- ctx.instance->tracing_on_init_val = read_tracing_on(ctx.instance);
+ ctx->instance->tracing_on_init_val = read_tracing_on(ctx->instance);
/* Some instances may not be created yet */
- if (ctx.instance->tracing_on_init_val < 0)
- ctx.instance->tracing_on_init_val = 1;
+ if (ctx->instance->tracing_on_init_val < 0)
+ ctx->instance->tracing_on_init_val = 1;
}

/* Extracting data records all events in the system. */
- if (IS_EXTRACT(&ctx) && !ctx.record_all)
+ if (IS_EXTRACT(ctx) && !ctx->record_all)
record_all_events();

- if (!IS_EXTRACT(&ctx))
+ if (!IS_EXTRACT(ctx))
make_instances();

- if (ctx.events)
+ if (ctx->events)
expand_event_list();

page_size = getpagesize();

- if (!IS_EXTRACT(&ctx)) {
- fset = set_ftrace(!ctx.disable, ctx.total_disable);
+ if (!IS_EXTRACT(ctx)) {
+ fset = set_ftrace(!ctx->disable, ctx->total_disable);
tracecmd_disable_all_tracing(1);

- for_all_instances(ctx.instance)
- set_clock(ctx.instance);
+ for_all_instances(ctx->instance)
+ set_clock(ctx->instance);

/* Record records the date first */
- if (IS_RECORD(&ctx) && ctx.date)
- ctx.date2ts = get_date_to_ts();
+ if (IS_RECORD(ctx) && ctx->date)
+ ctx->date2ts = get_date_to_ts();

- for_all_instances(ctx.instance) {
- set_funcs(ctx.instance);
- set_mask(ctx.instance);
+ for_all_instances(ctx->instance) {
+ set_funcs(ctx->instance);
+ set_mask(ctx->instance);
}

- if (ctx.events) {
- for_all_instances(ctx.instance)
- enable_events(ctx.instance);
+ if (ctx->events) {
+ for_all_instances(ctx->instance)
+ enable_events(ctx->instance);
}
set_buffer_size();
}

- if (IS_RECORD(&ctx))
+ if (IS_RECORD(ctx))
type = TRACE_TYPE_RECORD;
- else if (IS_STREAM(&ctx))
+ else if (IS_STREAM(ctx))
type = TRACE_TYPE_STREAM;
- else if (IS_EXTRACT(&ctx))
+ else if (IS_EXTRACT(ctx))
type = TRACE_TYPE_EXTRACT;
- else if (IS_PROFILE(&ctx))
+ else if (IS_PROFILE(ctx))
type = TRACE_TYPE_STREAM;
else
type = TRACE_TYPE_START;
@@ -4855,10 +4834,10 @@ void trace_record(int argc, char **argv)

set_options();

- if (ctx.max_graph_depth) {
- for_all_instances(ctx.instance)
- set_max_graph_depth(ctx.instance, ctx.max_graph_depth);
- free(ctx.max_graph_depth);
+ if (ctx->max_graph_depth) {
+ for_all_instances(ctx->instance)
+ set_max_graph_depth(ctx->instance, ctx->max_graph_depth);
+ free(ctx->max_graph_depth);
}

allocate_seq();
@@ -4866,10 +4845,10 @@ void trace_record(int argc, char **argv)
if (type & (TRACE_TYPE_RECORD | TRACE_TYPE_STREAM)) {
signal(SIGINT, finish);
if (!latency)
- start_threads(type, ctx.global);
+ start_threads(type, ctx->global);
}

- if (IS_EXTRACT(&ctx)) {
+ if (IS_EXTRACT(ctx)) {
flush_threads();

} else {
@@ -4879,7 +4858,7 @@ void trace_record(int argc, char **argv)
exit(0);
}

- if (ctx.run_command)
+ if (ctx->run_command)
run_cmd(type, (argc - optind) - 1, &argv[optind + 1]);
else {
update_task_filter();
@@ -4904,17 +4883,17 @@ void trace_record(int argc, char **argv)
tracecmd_disable_all_tracing(0);

/* extract records the date after extraction */
- if (IS_EXTRACT(&ctx) && ctx.date) {
+ if (IS_EXTRACT(ctx) && ctx->date) {
/*
* We need to start tracing, don't let other traces
* screw with our trace_marker.
*/
tracecmd_disable_all_tracing(1);
- ctx.date2ts = get_date_to_ts();
+ ctx->date2ts = get_date_to_ts();
}

- if (IS_RECORD(&ctx) || IS_EXTRACT(&ctx)) {
- record_data(ctx.date2ts, ctx.data_flags);
+ if (IS_RECORD(ctx) || IS_EXTRACT(ctx)) {
+ record_data(ctx->date2ts, ctx->data_flags);
delete_thread_data();
} else
print_stats();
@@ -4934,17 +4913,42 @@ void trace_record(int argc, char **argv)
tracecmd_remove_instances();

/* If tracing_on was enabled before we started, set it on now */
- for_all_instances(ctx.instance) {
- if (ctx.instance->keep)
- write_tracing_on(ctx.instance,
- ctx.instance->tracing_on_init_val);
+ for_all_instances(ctx->instance) {
+ if (ctx->instance->keep)
+ write_tracing_on(ctx->instance,
+ ctx->instance->tracing_on_init_val);
}

if (host)
tracecmd_output_close(network_handle);

- if (IS_PROFILE(&ctx))
+ if (IS_PROFILE(ctx))
trace_profile();

exit(0);
}
+
+void trace_record(int argc, char **argv)
+{
+ struct common_record_context ctx;
+
+ init_common_record_context(&ctx);
+
+ if (strcmp(argv[1], "record") == 0)
+ ctx.curr_cmd = CMD_record;
+ else if (strcmp(argv[1], "start") == 0)
+ ctx.curr_cmd = CMD_start;
+ else if (strcmp(argv[1], "extract") == 0)
+ ctx.curr_cmd = CMD_extract;
+ else if (strcmp(argv[1], "stream") == 0)
+ ctx.curr_cmd = CMD_stream;
+ else if (strcmp(argv[1], "profile") == 0) {
+ ctx.curr_cmd = CMD_profile;
+ handle_init = trace_init_profile;
+ ctx.events = 1;
+ } else
+ usage(argv);
+
+ parse_record_options(argc, argv, &ctx);
+ record_trace(argc, argv, &ctx);
+}
--
2.14.1


From 1584802790918744227@xxx Wed Nov 22 21:21:58 +0000 2017
X-GM-THRID: 1584802790918744227
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread