[PATCH 0/4] perf script: Add script per-event-dump support

[PATCH 0/4] perf script: Add script per-event-dump support

[yuzhoujian]

Introduce a new option to print trace output to files named by the
monitored events and update perf-script documentation accordingly.

Shown below is output of perf script command with the newly introduced
option.

        $perf record -e cycles -e context-switches -ag -- sleep 10
        $perf script --per-event-dump
        $ls /
        cycles.stacks context-switches.stacks

Without per-event-dump support, drawing flamegraphs for different events
is really hard. You can only monitor one event at a time for perf record.
Using this option, we can get the trace output files named by the monitored
events, and could draw flamegraphs according to the event's name.

yuzhoujian (4):
  Add a new element for the struct perf_tool, and add the
    --per-event-dump     option for perf script
  Makes all those related functions receive the FILE pointer
  Add the fp_selection_helper function to set the file pointer     for
    the related functions
  Replace printf with fprintf for all the output functions

 tools/perf/builtin-script.c | 451 +++++++++++++++++++++++++-------------------
 tools/perf/util/tool.h      |   1 +
 2 files changed, 253 insertions(+), 199 deletions(-)

-- 
1.8.3.1

[PATCH 1/4] Add a new element for the struct perf_tool, and add the --per-event-dump option for perf script

[yuzhoujian]

Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
---
 tools/perf/builtin-script.c | 11 ++++++++++-
 tools/perf/util/tool.h      |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 378f76c..91b5225 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -57,6 +57,7 @@
 static DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
 static struct perf_stat_config	stat_config;
 static int			max_blocks;
+static FILE			*per_event_dump_file;
 
 unsigned int scripting_max_stack = PERF_MAX_STACK_DEPTH;
 
@@ -2690,6 +2691,7 @@ int cmd_script(int argc, const char **argv)
 			.cpu_map	 = process_cpu_map_event,
 			.ordered_events	 = true,
 			.ordering_requires_timestamps = true,
+			.per_event_dump = false,
 		},
 	};
 	struct perf_data_file file = {
@@ -2760,6 +2762,8 @@ int cmd_script(int argc, const char **argv)
 		    "Show context switch events (if recorded)"),
 	OPT_BOOLEAN('\0', "show-namespace-events", &script.show_namespace_events,
 		    "Show namespace events (if recorded)"),
+	OPT_BOOLEAN('\0', "per-event-dump", &script.tool.per_event_dump,
+		    "print trace output to files named by the monitored events"),
 	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
 	OPT_INTEGER(0, "max-blocks", &max_blocks,
 		    "Maximum number of code blocks to dump with brstackinsn"),
@@ -2797,7 +2801,12 @@ int cmd_script(int argc, const char **argv)
 
 	file.path = input_name;
 	file.force = symbol_conf.force;
-
+	for (i = 1; argv[i] != NULL; i++) {
+		if (strcmp(argv[i], "--per-event-dump") == 0) {
+			script.tool.per_event_dump = true;
+			break;
+		}
+	}
 	if (argc > 1 && !strncmp(argv[0], "rec", strlen("rec"))) {
 		rec_script_path = get_script_path(argv[1], RECORD_SUFFIX);
 		if (!rec_script_path)
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index d549e50..ad40931 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -75,6 +75,7 @@ struct perf_tool {
 	bool		ordered_events;
 	bool		ordering_requires_timestamps;
 	bool		namespace_events;
+	bool		per_event_dump;
 	enum show_feature_header show_feat_hdr;
 };
 
-- 
1.8.3.1

Re: [PATCH 1/4] Add a new element for the struct perf_tool, and add the --per-event-dump option for perf script

[Jiri Olsa]

On Mon, Sep 18, 2017 at 01:55:19PM +0800, yuzhoujian wrote:
> Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>

missing changelog

jirka

> ---
>  tools/perf/builtin-script.c | 11 ++++++++++-
>  tools/perf/util/tool.h      |  1 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 378f76c..91b5225 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -57,6 +57,7 @@
>  static DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
>  static struct perf_stat_config	stat_config;
>  static int			max_blocks;
> +static FILE			*per_event_dump_file;
>  
>  unsigned int scripting_max_stack = PERF_MAX_STACK_DEPTH;
>  
> @@ -2690,6 +2691,7 @@ int cmd_script(int argc, const char **argv)
>  			.cpu_map	 = process_cpu_map_event,
>  			.ordered_events	 = true,
>  			.ordering_requires_timestamps = true,
> +			.per_event_dump = false,
>  		},
>  	};
>  	struct perf_data_file file = {
> @@ -2760,6 +2762,8 @@ int cmd_script(int argc, const char **argv)
>  		    "Show context switch events (if recorded)"),
>  	OPT_BOOLEAN('\0', "show-namespace-events", &script.show_namespace_events,
>  		    "Show namespace events (if recorded)"),
> +	OPT_BOOLEAN('\0', "per-event-dump", &script.tool.per_event_dump,
> +		    "print trace output to files named by the monitored events"),
>  	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
>  	OPT_INTEGER(0, "max-blocks", &max_blocks,
>  		    "Maximum number of code blocks to dump with brstackinsn"),
> @@ -2797,7 +2801,12 @@ int cmd_script(int argc, const char **argv)
>  
>  	file.path = input_name;
>  	file.force = symbol_conf.force;
> -
> +	for (i = 1; argv[i] != NULL; i++) {
> +		if (strcmp(argv[i], "--per-event-dump") == 0) {
> +			script.tool.per_event_dump = true;
> +			break;
> +		}
> +	}
>  	if (argc > 1 && !strncmp(argv[0], "rec", strlen("rec"))) {
>  		rec_script_path = get_script_path(argv[1], RECORD_SUFFIX);
>  		if (!rec_script_path)
> diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
> index d549e50..ad40931 100644
> --- a/tools/perf/util/tool.h
> +++ b/tools/perf/util/tool.h
> @@ -75,6 +75,7 @@ struct perf_tool {
>  	bool		ordered_events;
>  	bool		ordering_requires_timestamps;
>  	bool		namespace_events;
> +	bool		per_event_dump;
>  	enum show_feature_header show_feat_hdr;
>  };
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH 1/4] Add a new element for the struct perf_tool, and add the --per-event-dump option for perf script

[Jiri Olsa]

On Mon, Sep 18, 2017 at 01:55:19PM +0800, yuzhoujian wrote:
> Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
> ---
>  tools/perf/builtin-script.c | 11 ++++++++++-
>  tools/perf/util/tool.h      |  1 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 378f76c..91b5225 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -57,6 +57,7 @@
>  static DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
>  static struct perf_stat_config	stat_config;
>  static int			max_blocks;
> +static FILE			*per_event_dump_file;
>  
>  unsigned int scripting_max_stack = PERF_MAX_STACK_DEPTH;
>  
> @@ -2690,6 +2691,7 @@ int cmd_script(int argc, const char **argv)
>  			.cpu_map	 = process_cpu_map_event,
>  			.ordered_events	 = true,
>  			.ordering_requires_timestamps = true,
> +			.per_event_dump = false,
>  		},
>  	};
>  	struct perf_data_file file = {
> @@ -2760,6 +2762,8 @@ int cmd_script(int argc, const char **argv)
>  		    "Show context switch events (if recorded)"),
>  	OPT_BOOLEAN('\0', "show-namespace-events", &script.show_namespace_events,
>  		    "Show namespace events (if recorded)"),
> +	OPT_BOOLEAN('\0', "per-event-dump", &script.tool.per_event_dump,
> +		    "print trace output to files named by the monitored events"),
>  	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
>  	OPT_INTEGER(0, "max-blocks", &max_blocks,
>  		    "Maximum number of code blocks to dump with brstackinsn"),
> @@ -2797,7 +2801,12 @@ int cmd_script(int argc, const char **argv)
>  
>  	file.path = input_name;
>  	file.force = symbol_conf.force;
> -
> +	for (i = 1; argv[i] != NULL; i++) {
> +		if (strcmp(argv[i], "--per-event-dump") == 0) {
> +			script.tool.per_event_dump = true;
> +			break;
> +		}

why do you need this? OPT_BOOLEAN loads the per_event_dump

jirka

> +	}
>  	if (argc > 1 && !strncmp(argv[0], "rec", strlen("rec"))) {
>  		rec_script_path = get_script_path(argv[1], RECORD_SUFFIX);
>  		if (!rec_script_path)
> diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
> index d549e50..ad40931 100644
> --- a/tools/perf/util/tool.h
> +++ b/tools/perf/util/tool.h
> @@ -75,6 +75,7 @@ struct perf_tool {
>  	bool		ordered_events;
>  	bool		ordering_requires_timestamps;
>  	bool		namespace_events;
> +	bool		per_event_dump;
>  	enum show_feature_header show_feat_hdr;
>  };
>  
> -- 
> 1.8.3.1
> 

Re: [PATCH 1/4] Add a new element for the struct perf_tool, and add the --per-event-dump option for perf script

[Jiri Olsa]

On Mon, Sep 18, 2017 at 01:55:19PM +0800, yuzhoujian wrote:
> Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
> ---
>  tools/perf/builtin-script.c | 11 ++++++++++-
>  tools/perf/util/tool.h      |  1 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 378f76c..91b5225 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -57,6 +57,7 @@
>  static DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
>  static struct perf_stat_config	stat_config;
>  static int			max_blocks;
> +static FILE			*per_event_dump_file;

why is this added in this patch?

jirka

>  
>  unsigned int scripting_max_stack = PERF_MAX_STACK_DEPTH;
>  
> @@ -2690,6 +2691,7 @@ int cmd_script(int argc, const char **argv)
>  			.cpu_map	 = process_cpu_map_event,
>  			.ordered_events	 = true,
>  			.ordering_requires_timestamps = true,
> +			.per_event_dump = false,
>  		},
>  	};
>  	struct perf_data_file file = {
> @@ -2760,6 +2762,8 @@ int cmd_script(int argc, const char **argv)
>  		    "Show context switch events (if recorded)"),
>  	OPT_BOOLEAN('\0', "show-namespace-events", &script.show_namespace_events,
>  		    "Show namespace events (if recorded)"),
> +	OPT_BOOLEAN('\0', "per-event-dump", &script.tool.per_event_dump,
> +		    "print trace output to files named by the monitored events"),
>  	OPT_BOOLEAN('f', "force", &symbol_conf.force, "don't complain, do it"),
>  	OPT_INTEGER(0, "max-blocks", &max_blocks,
>  		    "Maximum number of code blocks to dump with brstackinsn"),
> @@ -2797,7 +2801,12 @@ int cmd_script(int argc, const char **argv)
>  
>  	file.path = input_name;
>  	file.force = symbol_conf.force;
> -
> +	for (i = 1; argv[i] != NULL; i++) {
> +		if (strcmp(argv[i], "--per-event-dump") == 0) {
> +			script.tool.per_event_dump = true;
> +			break;
> +		}
> +	}
>  	if (argc > 1 && !strncmp(argv[0], "rec", strlen("rec"))) {
>  		rec_script_path = get_script_path(argv[1], RECORD_SUFFIX);
>  		if (!rec_script_path)
> diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
> index d549e50..ad40931 100644
> --- a/tools/perf/util/tool.h
> +++ b/tools/perf/util/tool.h
> @@ -75,6 +75,7 @@ struct perf_tool {
>  	bool		ordered_events;
>  	bool		ordering_requires_timestamps;
>  	bool		namespace_events;
> +	bool		per_event_dump;
>  	enum show_feature_header show_feat_hdr;
>  };
>  
> -- 
> 1.8.3.1
> 

[PATCH 2/4] Makes all those related functions receive the FILE pointer

[yuzhoujian]

Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
---
 tools/perf/builtin-script.c | 247 ++++++++++++++++++++++++--------------------
 1 file changed, 136 insertions(+), 111 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 91b5225..f709f6f 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -486,8 +486,8 @@ static int perf_session__check_output_opt(struct perf_session *session)
 	return 0;
 }
 
-static void print_sample_iregs(struct perf_sample *sample,
-			  struct perf_event_attr *attr)
+static void fprint_sample_iregs(struct perf_sample *sample,
+			  struct perf_event_attr *attr, FILE *fp)
 {
 	struct regs_dump *regs = &sample->intr_regs;
 	uint64_t mask = attr->sample_regs_intr;
@@ -502,9 +502,9 @@ static void print_sample_iregs(struct perf_sample *sample,
 	}
 }
 
-static void print_sample_start(struct perf_sample *sample,
+static void fprint_sample_start(struct perf_sample *sample,
 			       struct thread *thread,
-			       struct perf_evsel *evsel)
+			       struct perf_evsel *evsel, FILE *fp)
 {
 	struct perf_event_attr *attr = &evsel->attr;
 	unsigned long secs;
@@ -557,9 +557,10 @@ static void print_sample_start(struct perf_sample *sample,
 	return br->flags.predicted ? 'P' : 'M';
 }
 
-static void print_sample_brstack(struct perf_sample *sample,
+static void fprint_sample_brstack(struct perf_sample *sample,
 				 struct thread *thread,
-				 struct perf_event_attr *attr)
+				 struct perf_event_attr *attr,
+				 FILE *fp)
 {
 	struct branch_stack *br = sample->branch_stack;
 	struct addr_location alf, alt;
@@ -582,14 +583,14 @@ static void print_sample_brstack(struct perf_sample *sample,
 		printf("0x%"PRIx64, from);
 		if (PRINT_FIELD(DSO)) {
 			printf("(");
-			map__fprintf_dsoname(alf.map, stdout);
+			map__fprintf_dsoname(alf.map, fp);
 			printf(")");
 		}
 
 		printf("/0x%"PRIx64, to);
 		if (PRINT_FIELD(DSO)) {
 			printf("(");
-			map__fprintf_dsoname(alt.map, stdout);
+			map__fprintf_dsoname(alt.map, fp);
 			printf(")");
 		}
 
@@ -601,9 +602,10 @@ static void print_sample_brstack(struct perf_sample *sample,
 	}
 }
 
-static void print_sample_brstacksym(struct perf_sample *sample,
+static void fprint_sample_brstacksym(struct perf_sample *sample,
 				    struct thread *thread,
-				    struct perf_event_attr *attr)
+				    struct perf_event_attr *attr,
+				    FILE *fp)
 {
 	struct branch_stack *br = sample->branch_stack;
 	struct addr_location alf, alt;
@@ -627,17 +629,17 @@ static void print_sample_brstacksym(struct perf_sample *sample,
 		if (alt.map)
 			alt.sym = map__find_symbol(alt.map, alt.addr);
 
-		symbol__fprintf_symname_offs(alf.sym, &alf, stdout);
+		symbol__fprintf_symname_offs(alf.sym, &alf, fp);
 		if (PRINT_FIELD(DSO)) {
 			printf("(");
-			map__fprintf_dsoname(alf.map, stdout);
+			map__fprintf_dsoname(alf.map, fp);
 			printf(")");
 		}
 		putchar('/');
-		symbol__fprintf_symname_offs(alt.sym, &alt, stdout);
+		symbol__fprintf_symname_offs(alt.sym, &alt, fp);
 		if (PRINT_FIELD(DSO)) {
 			printf("(");
-			map__fprintf_dsoname(alt.map, stdout);
+			map__fprintf_dsoname(alt.map, fp);
 			printf(")");
 		}
 		printf("/%c/%c/%c/%d ",
@@ -648,9 +650,10 @@ static void print_sample_brstacksym(struct perf_sample *sample,
 	}
 }
 
-static void print_sample_brstackoff(struct perf_sample *sample,
+static void fprint_sample_brstackoff(struct perf_sample *sample,
 				    struct thread *thread,
-				    struct perf_event_attr *attr)
+				    struct perf_event_attr *attr,
+				    FILE *fp)
 {
 	struct branch_stack *br = sample->branch_stack;
 	struct addr_location alf, alt;
@@ -677,13 +680,13 @@ static void print_sample_brstackoff(struct perf_sample *sample,
 		printf("0x%"PRIx64, from);
 		if (PRINT_FIELD(DSO)) {
 			printf("(");
-			map__fprintf_dsoname(alf.map, stdout);
+			map__fprintf_dsoname(alf.map, fp);
 			printf(")");
 		}
 		printf("/0x%"PRIx64, to);
 		if (PRINT_FIELD(DSO)) {
 			printf("(");
-			map__fprintf_dsoname(alt.map, stdout);
+			map__fprintf_dsoname(alt.map, fp);
 			printf(")");
 		}
 		printf("/%c/%c/%c/%d ",
@@ -757,9 +760,9 @@ static int grab_bb(u8 *buffer, u64 start, u64 end,
 	return len;
 }
 
-static void print_jump(uint64_t ip, struct branch_entry *en,
+static void fprint_jump(uint64_t ip, struct branch_entry *en,
 		       struct perf_insn *x, u8 *inbuf, int len,
-		       int insn)
+		       int insn, FILE *fp)
 {
 	printf("\t%016" PRIx64 "\t%-30s\t#%s%s%s%s",
 	       ip,
@@ -776,9 +779,9 @@ static void print_jump(uint64_t ip, struct branch_entry *en,
 	putchar('\n');
 }
 
-static void print_ip_sym(struct thread *thread, u8 cpumode, int cpu,
+static void fprint_ip_sym(struct thread *thread, u8 cpumode, int cpu,
 			 uint64_t addr, struct symbol **lastsym,
-			 struct perf_event_attr *attr)
+			 struct perf_event_attr *attr, FILE *fp)
 {
 	struct addr_location al;
 	int off;
@@ -809,15 +812,16 @@ static void print_ip_sym(struct thread *thread, u8 cpumode, int cpu,
 		printf("%+d", off);
 	putchar(':');
 	if (PRINT_FIELD(SRCLINE))
-		map__fprintf_srcline(al.map, al.addr, "\t", stdout);
+		map__fprintf_srcline(al.map, al.addr, "\t", fp);
 	putchar('\n');
 	*lastsym = al.sym;
 }
 
-static void print_sample_brstackinsn(struct perf_sample *sample,
+static void fprint_sample_brstackinsn(struct perf_sample *sample,
 				     struct thread *thread,
 				     struct perf_event_attr *attr,
-				     struct machine *machine)
+				     struct machine *machine,
+				     FILE *fp)
 {
 	struct branch_stack *br = sample->branch_stack;
 	u64 start, end;
@@ -843,10 +847,10 @@ static void print_sample_brstackinsn(struct perf_sample *sample,
 			br->entries[nr-1].from,
 			machine, thread, &x.is64bit, &x.cpumode, false);
 	if (len > 0) {
-		print_ip_sym(thread, x.cpumode, x.cpu,
-			     br->entries[nr - 1].from, &lastsym, attr);
-		print_jump(br->entries[nr - 1].from, &br->entries[nr - 1],
-			    &x, buffer, len, 0);
+		fprint_ip_sym(thread, x.cpumode, x.cpu,
+			     br->entries[nr - 1].from, &lastsym, attr, fp);
+		fprint_jump(br->entries[nr - 1].from, &br->entries[nr - 1],
+			    &x, buffer, len, 0, fp);
 	}
 
 	/* Print all blocks */
@@ -872,9 +876,9 @@ static void print_sample_brstackinsn(struct perf_sample *sample,
 		for (off = 0;; off += ilen) {
 			uint64_t ip = start + off;
 
-			print_ip_sym(thread, x.cpumode, x.cpu, ip, &lastsym, attr);
+			fprint_ip_sym(thread, x.cpumode, x.cpu, ip, &lastsym, attr, fp);
 			if (ip == end) {
-				print_jump(ip, &br->entries[i], &x, buffer + off, len - off, insn);
+				fprint_jump(ip, &br->entries[i], &x, buffer + off, len - off, insn, fp);
 				break;
 			} else {
 				printf("\t%016" PRIx64 "\t%s\n", ip,
@@ -901,7 +905,7 @@ static void print_sample_brstackinsn(struct perf_sample *sample,
 	start = br->entries[0].to;
 	end = sample->ip;
 	len = grab_bb(buffer, start, end, machine, thread, &x.is64bit, &x.cpumode, true);
-	print_ip_sym(thread, x.cpumode, x.cpu, start, &lastsym, attr);
+	fprint_ip_sym(thread, x.cpumode, x.cpu, start, &lastsym, attr, fp);
 	if (len <= 0) {
 		/* Print at least last IP if basic block did not work */
 		len = grab_bb(buffer, sample->ip, sample->ip,
@@ -921,9 +925,9 @@ static void print_sample_brstackinsn(struct perf_sample *sample,
 	}
 }
 
-static void print_sample_addr(struct perf_sample *sample,
+static void fprint_sample_addr(struct perf_sample *sample,
 			  struct thread *thread,
-			  struct perf_event_attr *attr)
+			  struct perf_event_attr *attr, FILE *fp)
 {
 	struct addr_location al;
 
@@ -937,22 +941,23 @@ static void print_sample_addr(struct perf_sample *sample,
 	if (PRINT_FIELD(SYM)) {
 		printf(" ");
 		if (PRINT_FIELD(SYMOFFSET))
-			symbol__fprintf_symname_offs(al.sym, &al, stdout);
+			symbol__fprintf_symname_offs(al.sym, &al, fp);
 		else
-			symbol__fprintf_symname(al.sym, stdout);
+			symbol__fprintf_symname(al.sym, fp);
 	}
 
 	if (PRINT_FIELD(DSO)) {
 		printf(" (");
-		map__fprintf_dsoname(al.map, stdout);
+		map__fprintf_dsoname(al.map, fp);
 		printf(")");
 	}
 }
 
-static void print_sample_callindent(struct perf_sample *sample,
+static void fprint_sample_callindent(struct perf_sample *sample,
 				    struct perf_evsel *evsel,
 				    struct thread *thread,
-				    struct addr_location *al)
+				    struct addr_location *al,
+				    FILE *fp)
 {
 	struct perf_event_attr *attr = &evsel->attr;
 	size_t depth = thread_stack__depth(thread);
@@ -1005,10 +1010,11 @@ static void print_sample_callindent(struct perf_sample *sample,
 		printf("%*s", spacing - len, "");
 }
 
-static void print_insn(struct perf_sample *sample,
+static void fprint_insn(struct perf_sample *sample,
 		       struct perf_event_attr *attr,
 		       struct thread *thread,
-		       struct machine *machine)
+		       struct machine *machine,
+		       FILE *fp)
 {
 	if (PRINT_FIELD(INSNLEN))
 		printf(" ilen: %d", sample->insn_len);
@@ -1020,21 +1026,22 @@ static void print_insn(struct perf_sample *sample,
 			printf(" %02x", (unsigned char)sample->insn[i]);
 	}
 	if (PRINT_FIELD(BRSTACKINSN))
-		print_sample_brstackinsn(sample, thread, attr, machine);
+		fprint_sample_brstackinsn(sample, thread, attr, machine, fp);
 }
 
-static void print_sample_bts(struct perf_sample *sample,
+static void fprint_sample_bts(struct perf_sample *sample,
 			     struct perf_evsel *evsel,
 			     struct thread *thread,
 			     struct addr_location *al,
-			     struct machine *machine)
+			     struct machine *machine,
+			     FILE *fp)
 {
 	struct perf_event_attr *attr = &evsel->attr;
 	unsigned int type = output_type(attr->type);
 	bool print_srcline_last = false;
 
 	if (PRINT_FIELD(CALLINDENT))
-		print_sample_callindent(sample, evsel, thread, al);
+		fprint_sample_callindent(sample, evsel, thread, al, fp);
 
 	/* print branch_from information */
 	if (PRINT_FIELD(IP)) {
@@ -1055,7 +1062,7 @@ static void print_sample_bts(struct perf_sample *sample,
 		} else
 			putchar('\n');
 
-		sample__fprintf_sym(sample, al, 0, print_opts, cursor, stdout);
+		sample__fprintf_sym(sample, al, 0, print_opts, cursor, fp);
 	}
 
 	/* print branch_to information */
@@ -1063,13 +1070,13 @@ static void print_sample_bts(struct perf_sample *sample,
 	    ((evsel->attr.sample_type & PERF_SAMPLE_ADDR) &&
 	     !output[type].user_set)) {
 		printf(" => ");
-		print_sample_addr(sample, thread, attr);
+		fprint_sample_addr(sample, thread, attr, fp);
 	}
 
 	if (print_srcline_last)
-		map__fprintf_srcline(al->map, al->addr, "\n  ", stdout);
+		map__fprintf_srcline(al->map, al->addr, "\n  ", fp);
 
-	print_insn(sample, attr, thread, machine);
+	fprint_insn(sample, attr, thread, machine, fp);
 
 	printf("\n");
 }
@@ -1094,7 +1101,7 @@ static void print_sample_bts(struct perf_sample *sample,
 	{0, NULL}
 };
 
-static void print_sample_flags(u32 flags)
+static void fprint_sample_flags(u32 flags, FILE *fp)
 {
 	const char *chars = PERF_IP_FLAG_CHARS;
 	const int n = strlen(PERF_IP_FLAG_CHARS);
@@ -1130,15 +1137,17 @@ struct printer_data {
 	int line_no;
 	bool hit_nul;
 	bool is_printable;
+	FILE *fp;
 };
 
 static void
-print_sample_bpf_output_printer(enum binary_printer_ops op,
+fprint_sample_bpf_output_printer(enum binary_printer_ops op,
 				unsigned int val,
 				void *extra)
 {
 	unsigned char ch = (unsigned char)val;
 	struct printer_data *printer_data = extra;
+	FILE *fp = printer_data->fp;
 
 	switch (op) {
 	case BINARY_PRINT_DATA_BEGIN:
@@ -1191,31 +1200,31 @@ struct printer_data {
 	}
 }
 
-static void print_sample_bpf_output(struct perf_sample *sample)
+static void fprint_sample_bpf_output(struct perf_sample *sample, FILE *fp)
 {
 	unsigned int nr_bytes = sample->raw_size;
-	struct printer_data printer_data = {0, false, true};
+	struct printer_data printer_data = {0, false, true, fp};
 
 	print_binary(sample->raw_data, nr_bytes, 8,
-		     print_sample_bpf_output_printer, &printer_data);
+		     fprint_sample_bpf_output_printer, &printer_data);
 
 	if (printer_data.is_printable && printer_data.hit_nul)
 		printf("%17s \"%s\"\n", "BPF string:",
 		       (char *)(sample->raw_data));
 }
 
-static void print_sample_spacing(int len, int spacing)
+static void fprint_sample_spacing(int len, int spacing, FILE *fp)
 {
 	if (len > 0 && len < spacing)
 		printf("%*s", spacing - len, "");
 }
 
-static void print_sample_pt_spacing(int len)
+static void fprint_sample_pt_spacing(int len, FILE *fp)
 {
-	print_sample_spacing(len, 34);
+	fprint_sample_spacing(len, 34, fp);
 }
 
-static void print_sample_synth_ptwrite(struct perf_sample *sample)
+static void fprint_sample_synth_ptwrite(struct perf_sample *sample, FILE *fp)
 {
 	struct perf_synth_intel_ptwrite *data = perf_sample__synth_ptr(sample);
 	int len;
@@ -1225,10 +1234,10 @@ static void print_sample_synth_ptwrite(struct perf_sample *sample)
 
 	len = printf(" IP: %u payload: %#" PRIx64 " ",
 		     data->ip, le64_to_cpu(data->payload));
-	print_sample_pt_spacing(len);
+	fprint_sample_pt_spacing(len, fp);
 }
 
-static void print_sample_synth_mwait(struct perf_sample *sample)
+static void fprint_sample_synth_mwait(struct perf_sample *sample, FILE *fp)
 {
 	struct perf_synth_intel_mwait *data = perf_sample__synth_ptr(sample);
 	int len;
@@ -1238,10 +1247,10 @@ static void print_sample_synth_mwait(struct perf_sample *sample)
 
 	len = printf(" hints: %#x extensions: %#x ",
 		     data->hints, data->extensions);
-	print_sample_pt_spacing(len);
+	fprint_sample_pt_spacing(len, fp);
 }
 
-static void print_sample_synth_pwre(struct perf_sample *sample)
+static void fprint_sample_synth_pwre(struct perf_sample *sample, FILE *fp)
 {
 	struct perf_synth_intel_pwre *data = perf_sample__synth_ptr(sample);
 	int len;
@@ -1251,10 +1260,10 @@ static void print_sample_synth_pwre(struct perf_sample *sample)
 
 	len = printf(" hw: %u cstate: %u sub-cstate: %u ",
 		     data->hw, data->cstate, data->subcstate);
-	print_sample_pt_spacing(len);
+	fprint_sample_pt_spacing(len, fp);
 }
 
-static void print_sample_synth_exstop(struct perf_sample *sample)
+static void fprint_sample_synth_exstop(struct perf_sample *sample, FILE *fp)
 {
 	struct perf_synth_intel_exstop *data = perf_sample__synth_ptr(sample);
 	int len;
@@ -1263,10 +1272,10 @@ static void print_sample_synth_exstop(struct perf_sample *sample)
 		return;
 
 	len = printf(" IP: %u ", data->ip);
-	print_sample_pt_spacing(len);
+	fprint_sample_pt_spacing(len, fp);
 }
 
-static void print_sample_synth_pwrx(struct perf_sample *sample)
+static void fprint_sample_synth_pwrx(struct perf_sample *sample, FILE *fp)
 {
 	struct perf_synth_intel_pwrx *data = perf_sample__synth_ptr(sample);
 	int len;
@@ -1277,10 +1286,10 @@ static void print_sample_synth_pwrx(struct perf_sample *sample)
 	len = printf(" deepest cstate: %u last cstate: %u wake reason: %#x ",
 		     data->deepest_cstate, data->last_cstate,
 		     data->wake_reason);
-	print_sample_pt_spacing(len);
+	fprint_sample_pt_spacing(len, fp);
 }
 
-static void print_sample_synth_cbr(struct perf_sample *sample)
+static void fprint_sample_synth_cbr(struct perf_sample *sample, FILE *fp)
 {
 	struct perf_synth_intel_cbr *data = perf_sample__synth_ptr(sample);
 	unsigned int percent, freq;
@@ -1295,30 +1304,30 @@ static void print_sample_synth_cbr(struct perf_sample *sample)
 		percent = (5 + (1000 * data->cbr) / data->max_nonturbo) / 10;
 		len += printf("(%3u%%) ", percent);
 	}
-	print_sample_pt_spacing(len);
+	fprint_sample_pt_spacing(len, fp);
 }
 
-static void print_sample_synth(struct perf_sample *sample,
-			       struct perf_evsel *evsel)
+static void fprint_sample_synth(struct perf_sample *sample,
+			       struct perf_evsel *evsel, FILE *fp)
 {
 	switch (evsel->attr.config) {
 	case PERF_SYNTH_INTEL_PTWRITE:
-		print_sample_synth_ptwrite(sample);
+		fprint_sample_synth_ptwrite(sample, fp);
 		break;
 	case PERF_SYNTH_INTEL_MWAIT:
-		print_sample_synth_mwait(sample);
+		fprint_sample_synth_mwait(sample, fp);
 		break;
 	case PERF_SYNTH_INTEL_PWRE:
-		print_sample_synth_pwre(sample);
+		fprint_sample_synth_pwre(sample, fp);
 		break;
 	case PERF_SYNTH_INTEL_EXSTOP:
-		print_sample_synth_exstop(sample);
+		fprint_sample_synth_exstop(sample, fp);
 		break;
 	case PERF_SYNTH_INTEL_PWRX:
-		print_sample_synth_pwrx(sample);
+		fprint_sample_synth_pwrx(sample, fp);
 		break;
 	case PERF_SYNTH_INTEL_CBR:
-		print_sample_synth_cbr(sample);
+		fprint_sample_synth_cbr(sample, fp);
 		break;
 	default:
 		break;
@@ -1354,7 +1363,7 @@ static int perf_evlist__max_name_len(struct perf_evlist *evlist)
 	return max;
 }
 
-static size_t data_src__printf(u64 data_src)
+static size_t data_src__fprintf(u64 data_src, FILE *fp)
 {
 	struct mem_info mi = { .data_src.val = data_src };
 	char decode[100];
@@ -1374,7 +1383,7 @@ static size_t data_src__printf(u64 data_src)
 static void process_event(struct perf_script *script,
 			  struct perf_sample *sample, struct perf_evsel *evsel,
 			  struct addr_location *al,
-			  struct machine *machine)
+			  struct machine *machine, FILE *fp)
 {
 	struct thread *thread = al->thread;
 	struct perf_event_attr *attr = &evsel->attr;
@@ -1383,7 +1392,7 @@ static void process_event(struct perf_script *script,
 	if (output[type].fields == 0)
 		return;
 
-	print_sample_start(sample, thread, evsel);
+	fprint_sample_start(sample, thread, evsel, fp);
 
 	if (PRINT_FIELD(PERIOD))
 		printf("%10" PRIu64 " ", sample->period);
@@ -1399,25 +1408,25 @@ static void process_event(struct perf_script *script,
 	}
 
 	if (print_flags)
-		print_sample_flags(sample->flags);
+		fprint_sample_flags(sample->flags, fp);
 
 	if (is_bts_event(attr)) {
-		print_sample_bts(sample, evsel, thread, al, machine);
+		fprint_sample_bts(sample, evsel, thread, al, machine, fp);
 		return;
 	}
 
 	if (PRINT_FIELD(TRACE))
-		event_format__print(evsel->tp_format, sample->cpu,
-				    sample->raw_data, sample->raw_size);
+		event_format__fprintf(evsel->tp_format, sample->cpu,
+				    sample->raw_data, sample->raw_size, fp);
 
 	if (attr->type == PERF_TYPE_SYNTH && PRINT_FIELD(SYNTH))
-		print_sample_synth(sample, evsel);
+		fprint_sample_synth(sample, evsel, fp);
 
 	if (PRINT_FIELD(ADDR))
-		print_sample_addr(sample, thread, attr);
+		fprint_sample_addr(sample, thread, attr, fp);
 
 	if (PRINT_FIELD(DATA_SRC))
-		data_src__printf(sample->data_src);
+		data_src__fprintf(sample->data_src, fp);
 
 	if (PRINT_FIELD(WEIGHT))
 		printf("%16" PRIu64, sample->weight);
@@ -1431,22 +1440,22 @@ static void process_event(struct perf_script *script,
 			cursor = &callchain_cursor;
 
 		putchar(cursor ? '\n' : ' ');
-		sample__fprintf_sym(sample, al, 0, output[type].print_ip_opts, cursor, stdout);
+		sample__fprintf_sym(sample, al, 0, output[type].print_ip_opts, cursor, fp);
 	}
 
 	if (PRINT_FIELD(IREGS))
-		print_sample_iregs(sample, attr);
+		fprint_sample_iregs(sample, attr, fp);
 
 	if (PRINT_FIELD(BRSTACK))
-		print_sample_brstack(sample, thread, attr);
+		fprint_sample_brstack(sample, thread, attr, fp);
 	else if (PRINT_FIELD(BRSTACKSYM))
-		print_sample_brstacksym(sample, thread, attr);
+		fprint_sample_brstacksym(sample, thread, attr, fp);
 	else if (PRINT_FIELD(BRSTACKOFF))
-		print_sample_brstackoff(sample, thread, attr);
+		fprint_sample_brstackoff(sample, thread, attr, fp);
 
 	if (perf_evsel__is_bpf_output(evsel) && PRINT_FIELD(BPF_OUTPUT))
-		print_sample_bpf_output(sample);
-	print_insn(sample, attr, thread, machine);
+		fprint_sample_bpf_output(sample, fp);
+	fprint_insn(sample, attr, thread, machine, fp);
 	printf("\n");
 }
 
@@ -1526,6 +1535,7 @@ static int process_sample_event(struct perf_tool *tool,
 {
 	struct perf_script *scr = container_of(tool, struct perf_script, tool);
 	struct addr_location al;
+	FILE *fp;
 
 	if (perf_time__skip_sample(&scr->ptime, sample->time))
 		return 0;
@@ -1555,8 +1565,10 @@ static int process_sample_event(struct perf_tool *tool,
 
 	if (scripting_ops)
 		scripting_ops->process_event(event, sample, evsel, &al);
-	else
-		process_event(scr, sample, evsel, &al, machine);
+	else {
+		fp = stdout;
+		process_event(scr, sample, evsel, &al, machine, fp);
+	}
 
 out_put:
 	addr_location__put(&al);
@@ -1605,6 +1617,7 @@ static int process_comm_event(struct perf_tool *tool,
 	struct perf_session *session = script->session;
 	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
 	int ret = -1;
+	FILE *fp;
 
 	thread = machine__findnew_thread(machine, event->comm.pid, event->comm.tid);
 	if (thread == NULL) {
@@ -1621,8 +1634,9 @@ static int process_comm_event(struct perf_tool *tool,
 		sample->tid = event->comm.tid;
 		sample->pid = event->comm.pid;
 	}
-	print_sample_start(sample, thread, evsel);
-	perf_event__fprintf(event, stdout);
+	fp = stdout;
+	fprint_sample_start(sample, thread, evsel, fp);
+	perf_event__fprintf(event, fp);
 	ret = 0;
 out:
 	thread__put(thread);
@@ -1639,6 +1653,7 @@ static int process_namespaces_event(struct perf_tool *tool,
 	struct perf_session *session = script->session;
 	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
 	int ret = -1;
+	FILE *fp;
 
 	thread = machine__findnew_thread(machine, event->namespaces.pid,
 					 event->namespaces.tid);
@@ -1656,8 +1671,9 @@ static int process_namespaces_event(struct perf_tool *tool,
 		sample->tid = event->namespaces.tid;
 		sample->pid = event->namespaces.pid;
 	}
-	print_sample_start(sample, thread, evsel);
-	perf_event__fprintf(event, stdout);
+	fp = stdout;
+	fprint_sample_start(sample, thread, evsel, fp);
+	perf_event__fprintf(event, fp);
 	ret = 0;
 out:
 	thread__put(thread);
@@ -1673,6 +1689,7 @@ static int process_fork_event(struct perf_tool *tool,
 	struct perf_script *script = container_of(tool, struct perf_script, tool);
 	struct perf_session *session = script->session;
 	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
+	FILE *fp;
 
 	if (perf_event__process_fork(tool, event, sample, machine) < 0)
 		return -1;
@@ -1689,8 +1706,9 @@ static int process_fork_event(struct perf_tool *tool,
 		sample->tid = event->fork.tid;
 		sample->pid = event->fork.pid;
 	}
-	print_sample_start(sample, thread, evsel);
-	perf_event__fprintf(event, stdout);
+	fp = stdout;
+	fprint_sample_start(sample, thread, evsel, fp);
+	perf_event__fprintf(event, fp);
 	thread__put(thread);
 
 	return 0;
@@ -1705,6 +1723,7 @@ static int process_exit_event(struct perf_tool *tool,
 	struct perf_script *script = container_of(tool, struct perf_script, tool);
 	struct perf_session *session = script->session;
 	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
+	FILE *fp;
 
 	thread = machine__findnew_thread(machine, event->fork.pid, event->fork.tid);
 	if (thread == NULL) {
@@ -1718,8 +1737,9 @@ static int process_exit_event(struct perf_tool *tool,
 		sample->tid = event->fork.tid;
 		sample->pid = event->fork.pid;
 	}
-	print_sample_start(sample, thread, evsel);
-	perf_event__fprintf(event, stdout);
+	fp = stdout;
+	fprint_sample_start(sample, thread, evsel, fp);
+	perf_event__fprintf(event, fp);
 
 	if (perf_event__process_exit(tool, event, sample, machine) < 0)
 		err = -1;
@@ -1737,6 +1757,7 @@ static int process_mmap_event(struct perf_tool *tool,
 	struct perf_script *script = container_of(tool, struct perf_script, tool);
 	struct perf_session *session = script->session;
 	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
+	FILE *fp;
 
 	if (perf_event__process_mmap(tool, event, sample, machine) < 0)
 		return -1;
@@ -1753,8 +1774,9 @@ static int process_mmap_event(struct perf_tool *tool,
 		sample->tid = event->mmap.tid;
 		sample->pid = event->mmap.pid;
 	}
-	print_sample_start(sample, thread, evsel);
-	perf_event__fprintf(event, stdout);
+	fp = stdout;
+	fprint_sample_start(sample, thread, evsel, fp);
+	perf_event__fprintf(event, fp);
 	thread__put(thread);
 	return 0;
 }
@@ -1768,6 +1790,7 @@ static int process_mmap2_event(struct perf_tool *tool,
 	struct perf_script *script = container_of(tool, struct perf_script, tool);
 	struct perf_session *session = script->session;
 	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
+	FILE *fp;
 
 	if (perf_event__process_mmap2(tool, event, sample, machine) < 0)
 		return -1;
@@ -1784,8 +1807,9 @@ static int process_mmap2_event(struct perf_tool *tool,
 		sample->tid = event->mmap2.tid;
 		sample->pid = event->mmap2.pid;
 	}
-	print_sample_start(sample, thread, evsel);
-	perf_event__fprintf(event, stdout);
+	fp = stdout;
+	fprint_sample_start(sample, thread, evsel, fp);
+	perf_event__fprintf(event, fp);
 	thread__put(thread);
 	return 0;
 }
@@ -1799,7 +1823,7 @@ static int process_switch_event(struct perf_tool *tool,
 	struct perf_script *script = container_of(tool, struct perf_script, tool);
 	struct perf_session *session = script->session;
 	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
-
+	FILE *fp;
 	if (perf_event__process_switch(tool, event, sample, machine) < 0)
 		return -1;
 
@@ -1810,8 +1834,9 @@ static int process_switch_event(struct perf_tool *tool,
 		return -1;
 	}
 
-	print_sample_start(sample, thread, evsel);
-	perf_event__fprintf(event, stdout);
+	fp = stdout;
+	fprint_sample_start(sample, thread, evsel, fp);
+	perf_event__fprintf(event, fp);
 	thread__put(thread);
 	return 0;
 }
-- 
1.8.3.1

Re: [PATCH 2/4] Makes all those related functions receive the FILE pointer

[Jiri Olsa]

On Mon, Sep 18, 2017 at 01:55:20PM +0800, yuzhoujian wrote:

SNIP

>  out_put:
>  	addr_location__put(&al);
> @@ -1605,6 +1617,7 @@ static int process_comm_event(struct perf_tool *tool,
>  	struct perf_session *session = script->session;
>  	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
>  	int ret = -1;
> +	FILE *fp;
>  
>  	thread = machine__findnew_thread(machine, event->comm.pid, event->comm.tid);
>  	if (thread == NULL) {
> @@ -1621,8 +1634,9 @@ static int process_comm_event(struct perf_tool *tool,
>  		sample->tid = event->comm.tid;
>  		sample->pid = event->comm.pid;
>  	}
> -	print_sample_start(sample, thread, evsel);
> -	perf_event__fprintf(event, stdout);
> +	fp = stdout;
> +	fprint_sample_start(sample, thread, evsel, fp);
> +	perf_event__fprintf(event, fp);

please make this simple 'adding fp argument to print functions'
change and just pass stdout in here ... and do the rest of changes
later on

thanks,
jirka

Re: [PATCH 2/4] Makes all those related functions receive the FILE pointer

[Arnaldo Carvalho de Melo]

Em Fri, Sep 22, 2017 at 11:03:12AM +0200, Jiri Olsa escreveu:
> On Mon, Sep 18, 2017 at 01:55:20PM +0800, yuzhoujian wrote:
> 
> SNIP
> 
> >  out_put:
> >  	addr_location__put(&al);
> > @@ -1605,6 +1617,7 @@ static int process_comm_event(struct perf_tool *tool,
> >  	struct perf_session *session = script->session;
> >  	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
> >  	int ret = -1;
> > +	FILE *fp;
> >  
> >  	thread = machine__findnew_thread(machine, event->comm.pid, event->comm.tid);
> >  	if (thread == NULL) {
> > @@ -1621,8 +1634,9 @@ static int process_comm_event(struct perf_tool *tool,
> >  		sample->tid = event->comm.tid;
> >  		sample->pid = event->comm.pid;
> >  	}
> > -	print_sample_start(sample, thread, evsel);
> > -	perf_event__fprintf(event, stdout);
> > +	fp = stdout;
> > +	fprint_sample_start(sample, thread, evsel, fp);
> > +	perf_event__fprintf(event, fp);
> 
> please make this simple 'adding fp argument to print functions'
> change and just pass stdout in here ... and do the rest of changes
> later on

That was what I asked him when I first reviewied this patchset.

- Arnaldo

Re: [PATCH 2/4] Makes all those related functions receive the FILE pointer

[Jiri Olsa]

On Mon, Sep 18, 2017 at 01:55:20PM +0800, yuzhoujian wrote:
> Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
> ---
>  tools/perf/builtin-script.c | 247 ++++++++++++++++++++++++--------------------
>  1 file changed, 136 insertions(+), 111 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 91b5225..f709f6f 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -486,8 +486,8 @@ static int perf_session__check_output_opt(struct perf_session *session)
>  	return 0;
>  }
>  
> -static void print_sample_iregs(struct perf_sample *sample,
> -			  struct perf_event_attr *attr)
> +static void fprint_sample_iregs(struct perf_sample *sample,
> +			  struct perf_event_attr *attr, FILE *fp)

please make sure the perf gets compiled after every change,
I think this will break compile because of unused *fp,
use the __maybe_unused attribute

jirka

Re: [PATCH 2/4] Makes all those related functions receive the FILE pointer

[Arnaldo Carvalho de Melo]

Em Fri, Sep 22, 2017 at 11:03:19AM +0200, Jiri Olsa escreveu:
> On Mon, Sep 18, 2017 at 01:55:20PM +0800, yuzhoujian wrote:
> > Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
> > +++ b/tools/perf/builtin-script.c
> > @@ -486,8 +486,8 @@ static int perf_session__check_output_opt(struct perf_session *session)
> >  	return 0;
> >  }
> >  
> > -static void print_sample_iregs(struct perf_sample *sample,
> > -			  struct perf_event_attr *attr)
> > +static void fprint_sample_iregs(struct perf_sample *sample,
> > +			  struct perf_event_attr *attr, FILE *fp)
> 
> please make sure the perf gets compiled after every change,
> I think this will break compile because of unused *fp,
> use the __maybe_unused attribute

Yes, that will save me some time, as I _will_ do this. Doing it
judiciously will make me want to review and process your patches faster
in the future, BTW.

To know why this is important, please read about 'git bisect'.

- Arnaldo

[PATCH 3/4] Add the fp_selection_helper function to set the file pointer for the related functions

[yuzhoujian]

Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
---
 tools/perf/builtin-script.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index f709f6f..89bab68 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1527,6 +1527,13 @@ static int cleanup_scripting(void)
 	return scripting_ops ? scripting_ops->stop_script() : 0;
 }
 
+static FILE *fp_selection_helper(bool per_event_dump)
+{
+	if (per_event_dump == false)
+		return stdout;
+	else
+		return per_event_dump_file;
+}
 static int process_sample_event(struct perf_tool *tool,
 				union perf_event *event,
 				struct perf_sample *sample,
@@ -1566,7 +1573,7 @@ static int process_sample_event(struct perf_tool *tool,
 	if (scripting_ops)
 		scripting_ops->process_event(event, sample, evsel, &al);
 	else {
-		fp = stdout;
+		fp = fp_selection_helper(tool->per_event_dump);
 		process_event(scr, sample, evsel, &al, machine, fp);
 	}
 
@@ -1634,7 +1641,7 @@ static int process_comm_event(struct perf_tool *tool,
 		sample->tid = event->comm.tid;
 		sample->pid = event->comm.pid;
 	}
-	fp = stdout;
+	fp = fp_selection_helper(tool->per_event_dump);
 	fprint_sample_start(sample, thread, evsel, fp);
 	perf_event__fprintf(event, fp);
 	ret = 0;
@@ -1671,7 +1678,7 @@ static int process_namespaces_event(struct perf_tool *tool,
 		sample->tid = event->namespaces.tid;
 		sample->pid = event->namespaces.pid;
 	}
-	fp = stdout;
+	fp = fp_selection_helper(tool->per_event_dump);
 	fprint_sample_start(sample, thread, evsel, fp);
 	perf_event__fprintf(event, fp);
 	ret = 0;
@@ -1706,7 +1713,7 @@ static int process_fork_event(struct perf_tool *tool,
 		sample->tid = event->fork.tid;
 		sample->pid = event->fork.pid;
 	}
-	fp = stdout;
+	fp = fp_selection_helper(tool->per_event_dump);
 	fprint_sample_start(sample, thread, evsel, fp);
 	perf_event__fprintf(event, fp);
 	thread__put(thread);
@@ -1737,7 +1744,7 @@ static int process_exit_event(struct perf_tool *tool,
 		sample->tid = event->fork.tid;
 		sample->pid = event->fork.pid;
 	}
-	fp = stdout;
+	fp = fp_selection_helper(tool->per_event_dump);
 	fprint_sample_start(sample, thread, evsel, fp);
 	perf_event__fprintf(event, fp);
 
@@ -1774,7 +1781,7 @@ static int process_mmap_event(struct perf_tool *tool,
 		sample->tid = event->mmap.tid;
 		sample->pid = event->mmap.pid;
 	}
-	fp = stdout;
+	fp = fp_selection_helper(tool->per_event_dump);
 	fprint_sample_start(sample, thread, evsel, fp);
 	perf_event__fprintf(event, fp);
 	thread__put(thread);
@@ -1807,7 +1814,7 @@ static int process_mmap2_event(struct perf_tool *tool,
 		sample->tid = event->mmap2.tid;
 		sample->pid = event->mmap2.pid;
 	}
-	fp = stdout;
+	fp = fp_selection_helper(tool->per_event_dump);
 	fprint_sample_start(sample, thread, evsel, fp);
 	perf_event__fprintf(event, fp);
 	thread__put(thread);
@@ -1824,6 +1831,7 @@ static int process_switch_event(struct perf_tool *tool,
 	struct perf_session *session = script->session;
 	struct perf_evsel *evsel = perf_evlist__id2evsel(session->evlist, sample->id);
 	FILE *fp;
+
 	if (perf_event__process_switch(tool, event, sample, machine) < 0)
 		return -1;
 
@@ -1834,7 +1842,7 @@ static int process_switch_event(struct perf_tool *tool,
 		return -1;
 	}
 
-	fp = stdout;
+	fp = fp_selection_helper(tool->per_event_dump);
 	fprint_sample_start(sample, thread, evsel, fp);
 	perf_event__fprintf(event, fp);
 	thread__put(thread);
-- 
1.8.3.1

Re: [PATCH 3/4] Add the fp_selection_helper function to set the file pointer for the related functions

[Jiri Olsa]

On Mon, Sep 18, 2017 at 01:55:21PM +0800, yuzhoujian wrote:
> Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
> ---
>  tools/perf/builtin-script.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index f709f6f..89bab68 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1527,6 +1527,13 @@ static int cleanup_scripting(void)
>  	return scripting_ops ? scripting_ops->stop_script() : 0;
>  }
>  
> +static FILE *fp_selection_helper(bool per_event_dump)
> +{
> +	if (per_event_dump == false)
> +		return stdout;
> +	else
> +		return per_event_dump_file;

when's the per_event_dump_file ever set? 

jirka

Re: [PATCH 3/4] Add the fp_selection_helper function to set the file pointer for the related functions

[Arnaldo Carvalho de Melo]

Em Fri, Sep 22, 2017 at 11:03:27AM +0200, Jiri Olsa escreveu:
> On Mon, Sep 18, 2017 at 01:55:21PM +0800, yuzhoujian wrote:
> > Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
> > ---
> >  tools/perf/builtin-script.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index f709f6f..89bab68 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -1527,6 +1527,13 @@ static int cleanup_scripting(void)
> >  	return scripting_ops ? scripting_ops->stop_script() : 0;
> >  }
> >  
> > +static FILE *fp_selection_helper(bool per_event_dump)
> > +{
> > +	if (per_event_dump == false)
> > +		return stdout;
> > +	else
> > +		return per_event_dump_file;
> 
> when's the per_event_dump_file ever set? 

And my first reaction with this "helper" was: what is this needed for?!?
Why not have a local variable in the function where the whole chain is
called from and there set the output doing this 'perf_event_dump' test
_just once_.

Also, using 'per_event_dump == false' for a boolean variable is valid,
but utterly uncommon and unnecessary, why not:

	FILE *fp = per_event_dump ? per_event_dump_file : stdout;

then use fp to pass to the initial function, etc?

- Arnaldo

[PATCH 4/4] Replace printf with fprintf for all the output functions

[yuzhoujian]

Signed-off-by: yuzhoujian <yuzhoujian@didichuxing.com>
---
 tools/perf/builtin-script.c | 187 +++++++++++++++++++++++---------------------
 1 file changed, 99 insertions(+), 88 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 89bab68..0d206b2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -498,7 +498,7 @@ static void fprint_sample_iregs(struct perf_sample *sample,
 
 	for_each_set_bit(r, (unsigned long *) &mask, sizeof(mask) * 8) {
 		u64 val = regs->regs[i++];
-		printf("%5s:0x%"PRIx64" ", perf_reg_name(r), val);
+		fprintf(fp, "%5s:0x%"PRIx64" ", perf_reg_name(r), val);
 	}
 }
 
@@ -512,25 +512,25 @@ static void fprint_sample_start(struct perf_sample *sample,
 
 	if (PRINT_FIELD(COMM)) {
 		if (latency_format)
-			printf("%8.8s ", thread__comm_str(thread));
+			fprintf(fp, "%8.8s ", thread__comm_str(thread));
 		else if (PRINT_FIELD(IP) && symbol_conf.use_callchain)
-			printf("%s ", thread__comm_str(thread));
+			fprintf(fp, "%s ", thread__comm_str(thread));
 		else
-			printf("%16s ", thread__comm_str(thread));
+			fprintf(fp, "%16s ", thread__comm_str(thread));
 	}
 
 	if (PRINT_FIELD(PID) && PRINT_FIELD(TID))
-		printf("%5d/%-5d ", sample->pid, sample->tid);
+		fprintf(fp, "%5d/%-5d ", sample->pid, sample->tid);
 	else if (PRINT_FIELD(PID))
-		printf("%5d ", sample->pid);
+		fprintf(fp, "%5d ", sample->pid);
 	else if (PRINT_FIELD(TID))
-		printf("%5d ", sample->tid);
+		fprintf(fp, "%5d ", sample->tid);
 
 	if (PRINT_FIELD(CPU)) {
 		if (latency_format)
-			printf("%3d ", sample->cpu);
+			fprintf(fp, "%3d ", sample->cpu);
 		else
-			printf("[%03d] ", sample->cpu);
+			fprintf(fp, "[%03d] ", sample->cpu);
 	}
 
 	if (PRINT_FIELD(TIME)) {
@@ -539,11 +539,11 @@ static void fprint_sample_start(struct perf_sample *sample,
 		nsecs -= secs * NSEC_PER_SEC;
 
 		if (nanosecs)
-			printf("%5lu.%09llu: ", secs, nsecs);
+			fprintf(fp, "%5lu.%09llu: ", secs, nsecs);
 		else {
 			char sample_time[32];
 			timestamp__scnprintf_usec(sample->time, sample_time, sizeof(sample_time));
-			printf("%12s: ", sample_time);
+			fprintf(fp, "%12s: ", sample_time);
 		}
 	}
 }
@@ -580,21 +580,21 @@ static void fprint_sample_brstack(struct perf_sample *sample,
 			thread__find_addr_map(thread, sample->cpumode, MAP__FUNCTION, to, &alt);
 		}
 
-		printf("0x%"PRIx64, from);
+		fprintf(fp, "0x%"PRIx64, from);
 		if (PRINT_FIELD(DSO)) {
-			printf("(");
+			fprintf(fp, "(");
 			map__fprintf_dsoname(alf.map, fp);
-			printf(")");
+			fprintf(fp, ")");
 		}
 
-		printf("/0x%"PRIx64, to);
+		fprintf(fp, "/0x%"PRIx64, to);
 		if (PRINT_FIELD(DSO)) {
-			printf("(");
+			fprintf(fp, "(");
 			map__fprintf_dsoname(alt.map, fp);
-			printf(")");
+			fprintf(fp, ")");
 		}
 
-		printf("/%c/%c/%c/%d ",
+		fprintf(fp, "/%c/%c/%c/%d ",
 			mispred_str( br->entries + i),
 			br->entries[i].flags.in_tx? 'X' : '-',
 			br->entries[i].flags.abort? 'A' : '-',
@@ -631,18 +631,18 @@ static void fprint_sample_brstacksym(struct perf_sample *sample,
 
 		symbol__fprintf_symname_offs(alf.sym, &alf, fp);
 		if (PRINT_FIELD(DSO)) {
-			printf("(");
+			fprintf(fp, "(");
 			map__fprintf_dsoname(alf.map, fp);
-			printf(")");
+			fprintf(fp, ")");
 		}
-		putchar('/');
+		fputc('/', fp);
 		symbol__fprintf_symname_offs(alt.sym, &alt, fp);
 		if (PRINT_FIELD(DSO)) {
-			printf("(");
+			fprintf(fp, "(");
 			map__fprintf_dsoname(alt.map, fp);
-			printf(")");
+			fprintf(fp, ")");
 		}
-		printf("/%c/%c/%c/%d ",
+		fprintf(fp, "/%c/%c/%c/%d ",
 			mispred_str( br->entries + i),
 			br->entries[i].flags.in_tx? 'X' : '-',
 			br->entries[i].flags.abort? 'A' : '-',
@@ -677,19 +677,19 @@ static void fprint_sample_brstackoff(struct perf_sample *sample,
 		if (alt.map && !alt.map->dso->adjust_symbols)
 			to = map__map_ip(alt.map, to);
 
-		printf("0x%"PRIx64, from);
+		fprintf(fp, "0x%"PRIx64, from);
 		if (PRINT_FIELD(DSO)) {
-			printf("(");
+			fprintf(fp, "(");
 			map__fprintf_dsoname(alf.map, fp);
-			printf(")");
+			fprintf(fp, ")");
 		}
-		printf("/0x%"PRIx64, to);
+		fprintf(fp, "/0x%"PRIx64, to);
 		if (PRINT_FIELD(DSO)) {
-			printf("(");
+			fprintf(fp, "(");
 			map__fprintf_dsoname(alt.map, fp);
-			printf(")");
+			fprintf(fp, ")");
 		}
-		printf("/%c/%c/%c/%d ",
+		fprintf(fp, "/%c/%c/%c/%d ",
 			mispred_str(br->entries + i),
 			br->entries[i].flags.in_tx ? 'X' : '-',
 			br->entries[i].flags.abort ? 'A' : '-',
@@ -764,7 +764,7 @@ static void fprint_jump(uint64_t ip, struct branch_entry *en,
 		       struct perf_insn *x, u8 *inbuf, int len,
 		       int insn, FILE *fp)
 {
-	printf("\t%016" PRIx64 "\t%-30s\t#%s%s%s%s",
+	fprintf(fp, "\t%016" PRIx64 "\t%-30s\t#%s%s%s%s",
 	       ip,
 	       dump_insn(x, ip, inbuf, len, NULL),
 	       en->flags.predicted ? " PRED" : "",
@@ -772,11 +772,11 @@ static void fprint_jump(uint64_t ip, struct branch_entry *en,
 	       en->flags.in_tx ? " INTX" : "",
 	       en->flags.abort ? " ABORT" : "");
 	if (en->flags.cycles) {
-		printf(" %d cycles", en->flags.cycles);
+		fprintf(fp, " %d cycles", en->flags.cycles);
 		if (insn)
-			printf(" %.2f IPC", (float)insn / en->flags.cycles);
+			fprintf(fp, " %.2f IPC", (float)insn / en->flags.cycles);
 	}
-	putchar('\n');
+	fputc('\n', fp);
 }
 
 static void fprint_ip_sym(struct thread *thread, u8 cpumode, int cpu,
@@ -807,13 +807,13 @@ static void fprint_ip_sym(struct thread *thread, u8 cpumode, int cpu,
 		off = al.addr - al.sym->start;
 	else
 		off = al.addr - al.map->start - al.sym->start;
-	printf("\t%s", al.sym->name);
+	fprintf(fp, "\t%s", al.sym->name);
 	if (off)
-		printf("%+d", off);
-	putchar(':');
+		fprintf(fp, "%+d", off);
+	fputc(':', fp);
 	if (PRINT_FIELD(SRCLINE))
 		map__fprintf_srcline(al.map, al.addr, "\t", fp);
-	putchar('\n');
+	fputc('\n', fp);
 	*lastsym = al.sym;
 }
 
@@ -840,7 +840,7 @@ static void fprint_sample_brstackinsn(struct perf_sample *sample,
 	x.thread = thread;
 	x.cpu = sample->cpu;
 
-	putchar('\n');
+	fputc('\n', fp);
 
 	/* Handle first from jump, of which we don't know the entry. */
 	len = grab_bb(buffer, br->entries[nr-1].from,
@@ -856,7 +856,7 @@ static void fprint_sample_brstackinsn(struct perf_sample *sample,
 	/* Print all blocks */
 	for (i = nr - 2; i >= 0; i--) {
 		if (br->entries[i].from || br->entries[i].to)
-			pr_debug("%d: %" PRIx64 "-%" PRIx64 "\n", i,
+			fprintf(fp, "%d: %" PRIx64 "-%" PRIx64 "\n", i,
 				 br->entries[i].from,
 				 br->entries[i].to);
 		start = br->entries[i + 1].to;
@@ -866,7 +866,7 @@ static void fprint_sample_brstackinsn(struct perf_sample *sample,
 		/* Patch up missing kernel transfers due to ring filters */
 		if (len == -ENXIO && i > 0) {
 			end = br->entries[--i].from;
-			pr_debug("\tpatching up to %" PRIx64 "-%" PRIx64 "\n", start, end);
+			fprintf(fp, "\tpatching up to %" PRIx64 "-%" PRIx64 "\n", start, end);
 			len = grab_bb(buffer, start, end, machine, thread, &x.is64bit, &x.cpumode, false);
 		}
 		if (len <= 0)
@@ -881,7 +881,7 @@ static void fprint_sample_brstackinsn(struct perf_sample *sample,
 				fprint_jump(ip, &br->entries[i], &x, buffer + off, len - off, insn, fp);
 				break;
 			} else {
-				printf("\t%016" PRIx64 "\t%s\n", ip,
+				fprintf(fp, "\t%016" PRIx64 "\t%s\n", ip,
 					dump_insn(&x, ip, buffer + off, len - off, &ilen));
 				if (ilen == 0)
 					break;
@@ -913,12 +913,12 @@ static void fprint_sample_brstackinsn(struct perf_sample *sample,
 		if (len <= 0)
 			return;
 
-		printf("\t%016" PRIx64 "\t%s\n", sample->ip,
+		fprintf(fp, "\t%016" PRIx64 "\t%s\n", sample->ip,
 			dump_insn(&x, sample->ip, buffer, len, NULL));
 		return;
 	}
 	for (off = 0; off <= end - start; off += ilen) {
-		printf("\t%016" PRIx64 "\t%s\n", start + off,
+		fprintf(fp, "\t%016" PRIx64 "\t%s\n", start + off,
 			dump_insn(&x, start + off, buffer + off, len - off, &ilen));
 		if (ilen == 0)
 			break;
@@ -931,7 +931,7 @@ static void fprint_sample_addr(struct perf_sample *sample,
 {
 	struct addr_location al;
 
-	printf("%16" PRIx64, sample->addr);
+	fprintf(fp, "%16" PRIx64, sample->addr);
 
 	if (!sample_addr_correlates_sym(attr))
 		return;
@@ -939,7 +939,7 @@ static void fprint_sample_addr(struct perf_sample *sample,
 	thread__resolve(thread, &al, sample);
 
 	if (PRINT_FIELD(SYM)) {
-		printf(" ");
+		fprintf(fp, " ");
 		if (PRINT_FIELD(SYMOFFSET))
 			symbol__fprintf_symname_offs(al.sym, &al, fp);
 		else
@@ -947,9 +947,9 @@ static void fprint_sample_addr(struct perf_sample *sample,
 	}
 
 	if (PRINT_FIELD(DSO)) {
-		printf(" (");
+		fprintf(fp, " (");
 		map__fprintf_dsoname(al.map, fp);
-		printf(")");
+		fprintf(fp, ")");
 	}
 }
 
@@ -992,9 +992,9 @@ static void fprint_sample_callindent(struct perf_sample *sample,
 	}
 
 	if (name)
-		len = printf("%*s%s", (int)depth * 4, "", name);
+		len = fprintf(fp, "%*s%s", (int)depth * 4, "", name);
 	else if (ip)
-		len = printf("%*s%16" PRIx64, (int)depth * 4, "", ip);
+		len = fprintf(fp, "%*s%16" PRIx64, (int)depth * 4, "", ip);
 
 	if (len < 0)
 		return;
@@ -1007,7 +1007,7 @@ static void fprint_sample_callindent(struct perf_sample *sample,
 		spacing = round_up(len + 4, 32);
 
 	if (len < spacing)
-		printf("%*s", spacing - len, "");
+		fprintf(fp, "%*s", spacing - len, "");
 }
 
 static void fprint_insn(struct perf_sample *sample,
@@ -1017,13 +1017,13 @@ static void fprint_insn(struct perf_sample *sample,
 		       FILE *fp)
 {
 	if (PRINT_FIELD(INSNLEN))
-		printf(" ilen: %d", sample->insn_len);
+		fprintf(fp, " ilen: %d", sample->insn_len);
 	if (PRINT_FIELD(INSN)) {
 		int i;
 
-		printf(" insn:");
+		fprintf(fp, " insn:");
 		for (i = 0; i < sample->insn_len; i++)
-			printf(" %02x", (unsigned char)sample->insn[i]);
+			fprintf(fp, " %02x", (unsigned char)sample->insn[i]);
 	}
 	if (PRINT_FIELD(BRSTACKINSN))
 		fprint_sample_brstackinsn(sample, thread, attr, machine, fp);
@@ -1054,13 +1054,13 @@ static void fprint_sample_bts(struct perf_sample *sample,
 			cursor = &callchain_cursor;
 
 		if (cursor == NULL) {
-			putchar(' ');
+			fputc(' ', fp);
 			if (print_opts & EVSEL__PRINT_SRCLINE) {
 				print_srcline_last = true;
 				print_opts &= ~EVSEL__PRINT_SRCLINE;
 			}
 		} else
-			putchar('\n');
+			fputc('\n', fp);
 
 		sample__fprintf_sym(sample, al, 0, print_opts, cursor, fp);
 	}
@@ -1069,7 +1069,7 @@ static void fprint_sample_bts(struct perf_sample *sample,
 	if (PRINT_FIELD(ADDR) ||
 	    ((evsel->attr.sample_type & PERF_SAMPLE_ADDR) &&
 	     !output[type].user_set)) {
-		printf(" => ");
+		fprintf(fp, " => ");
 		fprint_sample_addr(sample, thread, attr, fp);
 	}
 
@@ -1078,7 +1078,7 @@ static void fprint_sample_bts(struct perf_sample *sample,
 
 	fprint_insn(sample, attr, thread, machine, fp);
 
-	printf("\n");
+	fprintf(fp, "\n");
 }
 
 static struct {
@@ -1128,9 +1128,9 @@ static void fprint_sample_flags(u32 flags, FILE *fp)
 	str[pos] = 0;
 
 	if (name)
-		printf("  %-7s%4s ", name, in_tx ? "(x)" : "");
+		fprintf(fp, "  %-7s%4s ", name, in_tx ? "(x)" : "");
 	else
-		printf("  %-11s ", str);
+		fprintf(fp, "  %-11s ", str);
 }
 
 struct printer_data {
@@ -1151,30 +1151,30 @@ struct printer_data {
 
 	switch (op) {
 	case BINARY_PRINT_DATA_BEGIN:
-		printf("\n");
+		fprintf(fp, "\n");
 		break;
 	case BINARY_PRINT_LINE_BEGIN:
-		printf("%17s", !printer_data->line_no ? "BPF output:" :
+		fprintf(fp, "%17s", !printer_data->line_no ? "BPF output:" :
 						        "           ");
 		break;
 	case BINARY_PRINT_ADDR:
-		printf(" %04x:", val);
+		fprintf(fp, " %04x:", val);
 		break;
 	case BINARY_PRINT_NUM_DATA:
-		printf(" %02x", val);
+		fprintf(fp, " %02x", val);
 		break;
 	case BINARY_PRINT_NUM_PAD:
-		printf("   ");
+		fprintf(fp, "   ");
 		break;
 	case BINARY_PRINT_SEP:
-		printf("  ");
+		fprintf(fp, "  ");
 		break;
 	case BINARY_PRINT_CHAR_DATA:
 		if (printer_data->hit_nul && ch)
 			printer_data->is_printable = false;
 
 		if (!isprint(ch)) {
-			printf("%c", '.');
+			fprintf(fp, "%c", '.');
 
 			if (!printer_data->is_printable)
 				break;
@@ -1184,14 +1184,14 @@ struct printer_data {
 			else
 				printer_data->is_printable = false;
 		} else {
-			printf("%c", ch);
+			fprintf(fp, "%c", ch);
 		}
 		break;
 	case BINARY_PRINT_CHAR_PAD:
-		printf(" ");
+		fprintf(fp, " ");
 		break;
 	case BINARY_PRINT_LINE_END:
-		printf("\n");
+		fprintf(fp, "\n");
 		printer_data->line_no++;
 		break;
 	case BINARY_PRINT_DATA_END:
@@ -1209,14 +1209,14 @@ static void fprint_sample_bpf_output(struct perf_sample *sample, FILE *fp)
 		     fprint_sample_bpf_output_printer, &printer_data);
 
 	if (printer_data.is_printable && printer_data.hit_nul)
-		printf("%17s \"%s\"\n", "BPF string:",
+		fprintf(fp, "%17s \"%s\"\n", "BPF string:",
 		       (char *)(sample->raw_data));
 }
 
 static void fprint_sample_spacing(int len, int spacing, FILE *fp)
 {
 	if (len > 0 && len < spacing)
-		printf("%*s", spacing - len, "");
+		fprintf(fp, "%*s", spacing - len, "");
 }
 
 static void fprint_sample_pt_spacing(int len, FILE *fp)
@@ -1232,7 +1232,7 @@ static void fprint_sample_synth_ptwrite(struct perf_sample *sample, FILE *fp)
 	if (perf_sample__bad_synth_size(sample, *data))
 		return;
 
-	len = printf(" IP: %u payload: %#" PRIx64 " ",
+	len = fprintf(fp, " IP: %u payload: %#" PRIx64 " ",
 		     data->ip, le64_to_cpu(data->payload));
 	fprint_sample_pt_spacing(len, fp);
 }
@@ -1245,7 +1245,7 @@ static void fprint_sample_synth_mwait(struct perf_sample *sample, FILE *fp)
 	if (perf_sample__bad_synth_size(sample, *data))
 		return;
 
-	len = printf(" hints: %#x extensions: %#x ",
+	len = fprintf(fp, " hints: %#x extensions: %#x ",
 		     data->hints, data->extensions);
 	fprint_sample_pt_spacing(len, fp);
 }
@@ -1258,7 +1258,7 @@ static void fprint_sample_synth_pwre(struct perf_sample *sample, FILE *fp)
 	if (perf_sample__bad_synth_size(sample, *data))
 		return;
 
-	len = printf(" hw: %u cstate: %u sub-cstate: %u ",
+	len = fprintf(fp, " hw: %u cstate: %u sub-cstate: %u ",
 		     data->hw, data->cstate, data->subcstate);
 	fprint_sample_pt_spacing(len, fp);
 }
@@ -1271,7 +1271,7 @@ static void fprint_sample_synth_exstop(struct perf_sample *sample, FILE *fp)
 	if (perf_sample__bad_synth_size(sample, *data))
 		return;
 
-	len = printf(" IP: %u ", data->ip);
+	len = fprintf(fp, " IP: %u ", data->ip);
 	fprint_sample_pt_spacing(len, fp);
 }
 
@@ -1283,7 +1283,7 @@ static void fprint_sample_synth_pwrx(struct perf_sample *sample, FILE *fp)
 	if (perf_sample__bad_synth_size(sample, *data))
 		return;
 
-	len = printf(" deepest cstate: %u last cstate: %u wake reason: %#x ",
+	len = fprintf(fp, " deepest cstate: %u last cstate: %u wake reason: %#x ",
 		     data->deepest_cstate, data->last_cstate,
 		     data->wake_reason);
 	fprint_sample_pt_spacing(len, fp);
@@ -1299,10 +1299,10 @@ static void fprint_sample_synth_cbr(struct perf_sample *sample, FILE *fp)
 		return;
 
 	freq = (le32_to_cpu(data->freq) + 500) / 1000;
-	len = printf(" cbr: %2u freq: %4u MHz ", data->cbr, freq);
+	len = fprintf(fp, " cbr: %2u freq: %4u MHz ", data->cbr, freq);
 	if (data->max_nonturbo) {
 		percent = (5 + (1000 * data->cbr) / data->max_nonturbo) / 10;
-		len += printf("(%3u%%) ", percent);
+		len += fprintf(fp, "(%3u%%) ", percent);
 	}
 	fprint_sample_pt_spacing(len, fp);
 }
@@ -1377,7 +1377,7 @@ static size_t data_src__fprintf(u64 data_src, FILE *fp)
 	if (maxlen < len)
 		maxlen = len;
 
-	return printf("%-*s", maxlen, out);
+	return fprintf(fp, "%-*s", maxlen, out);
 }
 
 static void process_event(struct perf_script *script,
@@ -1389,21 +1389,30 @@ static void process_event(struct perf_script *script,
 	struct perf_event_attr *attr = &evsel->attr;
 	unsigned int type = output_type(attr->type);
 
+	const char *evname;
+	char *file_name;
 	if (output[type].fields == 0)
 		return;
 
+	evname = perf_evsel__name(evsel);
+	if (script->tool.per_event_dump == true) {
+		if (asprintf(&file_name, "%s%s", evname, ".stack") < 0)
+			file_name = NULL;
+		fp = fopen(file_name, "a+");
+		if (!fp)
+			return;
+		free(file_name);
+	}
 	fprint_sample_start(sample, thread, evsel, fp);
 
 	if (PRINT_FIELD(PERIOD))
-		printf("%10" PRIu64 " ", sample->period);
+		fprintf(fp, "%10" PRIu64 " ", sample->period);
 
 	if (PRINT_FIELD(EVNAME)) {
-		const char *evname = perf_evsel__name(evsel);
-
 		if (!script->name_width)
 			script->name_width = perf_evlist__max_name_len(script->session->evlist);
 
-		printf("%*s: ", script->name_width,
+		fprintf(fp, "%*s: ", script->name_width,
 		       evname ? evname : "[unknown]");
 	}
 
@@ -1429,7 +1438,7 @@ static void process_event(struct perf_script *script,
 		data_src__fprintf(sample->data_src, fp);
 
 	if (PRINT_FIELD(WEIGHT))
-		printf("%16" PRIu64, sample->weight);
+		fprintf(fp, "%16" PRIu64, sample->weight);
 
 	if (PRINT_FIELD(IP)) {
 		struct callchain_cursor *cursor = NULL;
@@ -1439,7 +1448,7 @@ static void process_event(struct perf_script *script,
 					      sample, NULL, NULL, scripting_max_stack) == 0)
 			cursor = &callchain_cursor;
 
-		putchar(cursor ? '\n' : ' ');
+		fputc(cursor ? '\n' : ' ', fp);
 		sample__fprintf_sym(sample, al, 0, output[type].print_ip_opts, cursor, fp);
 	}
 
@@ -1456,7 +1465,9 @@ static void process_event(struct perf_script *script,
 	if (perf_evsel__is_bpf_output(evsel) && PRINT_FIELD(BPF_OUTPUT))
 		fprint_sample_bpf_output(sample, fp);
 	fprint_insn(sample, attr, thread, machine, fp);
-	printf("\n");
+	fprintf(fp, "\n");
+	if (script->tool.per_event_dump == true)
+		fclose(fp);
 }
 
 static struct scripting_ops	*scripting_ops;
-- 
1.8.3.1

Re: [PATCH 4/4] Replace printf with fprintf for all the output functions

[Jiri Olsa]

On Mon, Sep 18, 2017 at 01:55:22PM +0800, yuzhoujian wrote:

SNIP

>  static void process_event(struct perf_script *script,
> @@ -1389,21 +1389,30 @@ static void process_event(struct perf_script *script,
>  	struct perf_event_attr *attr = &evsel->attr;
>  	unsigned int type = output_type(attr->type);
>  
> +	const char *evname;
> +	char *file_name;
>  	if (output[type].fields == 0)
>  		return;
>  
> +	evname = perf_evsel__name(evsel);
> +	if (script->tool.per_event_dump == true) {
> +		if (asprintf(&file_name, "%s%s", evname, ".stack") < 0)

what's the 'stack' suffix for? It's text dump, should we use .txt?
Also I think it should be more than 'cycles.stack', more like:

  <ORIGINAL PERF DATA FILE NAME>-script-dump-cycles.txt

or something like this

Arnaldo, thoughts?

thanks,
jirka

Re: [PATCH 4/4] Replace printf with fprintf for all the output functions

[Arnaldo Carvalho de Melo]

Em Fri, Sep 22, 2017 at 11:03:04AM +0200, Jiri Olsa escreveu:
> On Mon, Sep 18, 2017 at 01:55:22PM +0800, yuzhoujian wrote:
> 
> SNIP
> 
> >  static void process_event(struct perf_script *script,
> > @@ -1389,21 +1389,30 @@ static void process_event(struct perf_script *script,
> >  	struct perf_event_attr *attr = &evsel->attr;
> >  	unsigned int type = output_type(attr->type);
> >  
> > +	const char *evname;
> > +	char *file_name;
> >  	if (output[type].fields == 0)
> >  		return;
> >  
> > +	evname = perf_evsel__name(evsel);
> > +	if (script->tool.per_event_dump == true) {
> > +		if (asprintf(&file_name, "%s%s", evname, ".stack") < 0)
> 
> what's the 'stack' suffix for? It's text dump, should we use .txt?
> Also I think it should be more than 'cycles.stack', more like:
> 
>   <ORIGINAL PERF DATA FILE NAME>-script-dump-cycles.txt
> 
> or something like this
> 
> Arnaldo, thoughts?

Looks better, yes, I also was puzzled with that ".stack" suffix. And
your comment about using as a prefix the perf.data file being processed
also looks really needed.

- Arnaldo

Re: [PATCH 4/4] Replace printf with fprintf for all the output functions

[Jiri Olsa]

On Mon, Sep 18, 2017 at 01:55:22PM +0800, yuzhoujian wrote:

SNIP

>  		     data->deepest_cstate, data->last_cstate,
>  		     data->wake_reason);
>  	fprint_sample_pt_spacing(len, fp);
> @@ -1299,10 +1299,10 @@ static void fprint_sample_synth_cbr(struct perf_sample *sample, FILE *fp)
>  		return;
>  
>  	freq = (le32_to_cpu(data->freq) + 500) / 1000;
> -	len = printf(" cbr: %2u freq: %4u MHz ", data->cbr, freq);
> +	len = fprintf(fp, " cbr: %2u freq: %4u MHz ", data->cbr, freq);
>  	if (data->max_nonturbo) {
>  		percent = (5 + (1000 * data->cbr) / data->max_nonturbo) / 10;
> -		len += printf("(%3u%%) ", percent);
> +		len += fprintf(fp, "(%3u%%) ", percent);
>  	}
>  	fprint_sample_pt_spacing(len, fp);
>  }
> @@ -1377,7 +1377,7 @@ static size_t data_src__fprintf(u64 data_src, FILE *fp)
>  	if (maxlen < len)
>  		maxlen = len;
>  
> -	return printf("%-*s", maxlen, out);
> +	return fprintf(fp, "%-*s", maxlen, out);

all these 'fp' adding changes should be in separate patch

jirka

Re: [PATCH 0/4] perf script: Add script per-event-dump support

[Jiri Olsa]

On Mon, Sep 18, 2017 at 01:55:18PM +0800, yuzhoujian wrote:
> Introduce a new option to print trace output to files named by the
> monitored events and update perf-script documentation accordingly.
> 
> Shown below is output of perf script command with the newly introduced
> option.
> 
>         $perf record -e cycles -e context-switches -ag -- sleep 10
>         $perf script --per-event-dump
>         $ls /
>         cycles.stacks context-switches.stacks

hum, how adding -e option to be consistent with record, like:

	$ perf record -e cycles -e context-switches ...

	$ perf script -e cycles
	--- dumps only cycles to perf.data

	$ perf report -e cycles 
	--- displays only cycles

thanks,
jirka