From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753157AbdKWQge (ORCPT ); Thu, 23 Nov 2017 11:36:34 -0500 Received: from mail-wm0-f43.google.com ([74.125.82.43]:35820 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753191AbdKWQeH (ORCPT ); Thu, 23 Nov 2017 11:34:07 -0500 X-Google-Smtp-Source: AGs4zMZhr+9PBplzvEMZC8qCgA/OPCH7h/zRIS53c5R5n7BkNHcat+z82zz3wJPoIlAKburb4zFYOw== From: "Vladislav Valtchev (VMware)" To: rostedt@goodmis.org Cc: linux-kernel@vger.kernel.org, y.karadz@gmail.com, "Vladislav Valtchev (VMware)" Subject: [PATCH 04/11] trace-cmd: Extract parse_record_options() from trace_record() Date: Thu, 23 Nov 2017 18:33:28 +0200 Message-Id: <20171123163335.19078-5-vladislav.valtchev@gmail.com> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20171123163335.19078-1-vladislav.valtchev@gmail.com> References: <20171123163335.19078-1-vladislav.valtchev@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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) --- trace-record.c | 336 ++++++++++++++++++++++++++++++--------------------------- 1 file changed, 180 insertions(+), 156 deletions(-) diff --git a/trace-record.c b/trace-record.c index 8f8d270..6dc828b 100644 --- a/trace-record.c +++ b/trace-record.c @@ -4358,62 +4358,50 @@ 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 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 +4419,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 +4431,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 +4483,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 +4506,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 +4542,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 +4583,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 +4598,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 +4647,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 +4660,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 +4675,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,106 +4705,141 @@ 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; } +} + +static void init_common_record_context(struct common_record_context *ctx) +{ + memset(ctx, 0, sizeof(*ctx)); + ctx->instance = &top_instance; + cpu_count = count_cpus(); +} + +void trace_record(int argc, char **argv) +{ + struct common_record_context ctx; + enum trace_type type = 0; + + init_common_record_context(&ctx); + init_instance(ctx.instance); + + 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)) { if (!buffer_instances) die("No instances reference??"); 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; @@ -4825,10 +4848,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(); @@ -4836,10 +4859,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 { @@ -4849,7 +4872,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(); @@ -4874,17 +4897,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(); @@ -4904,15 +4927,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