[PATCH 0/5] perf: Clean up by adding helpers and Merging

[PATCH 0/5] perf: Clean up by adding helpers and Merging

[Shang XiaoJing]

Some Clean up in util.

Shang XiaoJing (5):
  perf block-info: Merge branches in __block_info__cmp
  perf block-range: Add block_range__insert_XXX helper
  perf dso: Merge cases in dso__read_binary_type_filename
  perf stat: Add print_cache_common helper
  perf genelf: Add create_section_and_get_data helper

 tools/perf/util/block-info.c  |  4 +-
 tools/perf/util/block-range.c | 36 +++++++------
 tools/perf/util/dso.c         |  7 +--
 tools/perf/util/genelf.c      | 96 +++++++++--------------------------
 tools/perf/util/stat-shadow.c | 80 +++++++++--------------------
 5 files changed, 74 insertions(+), 149 deletions(-)

-- 
2.17.1

[PATCH 1/5] perf block-info: Merge branches in __block_info__cmp

[Shang XiaoJing]

Merge two branches in __block_info__cmp into one, whiches have same
returning value.

Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
 tools/perf/util/block-info.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/perf/util/block-info.c b/tools/perf/util/block-info.c
index 5ecd4f401f32..e901bb6c88f8 100644
--- a/tools/perf/util/block-info.c
+++ b/tools/perf/util/block-info.c
@@ -72,9 +72,7 @@ int64_t __block_info__cmp(struct hist_entry *left, struct hist_entry *right)
 	int cmp;
 
 	if (!bi_l->sym || !bi_r->sym) {
-		if (!bi_l->sym && !bi_r->sym)
-			return -1;
-		else if (!bi_l->sym)
+		if (!bi_l->sym)
 			return -1;
 		else
 			return 1;
-- 
2.17.1

[PATCH 2/5] perf block-range: Add block_range__insert_XXX helper

[Shang XiaoJing]

Wrap repeated and correlated code in helper function
block_range__insert_left and block_range__insert_right, which used for
inserting the new block_range into rb_tree.

Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
 tools/perf/util/block-range.c | 36 ++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/block-range.c b/tools/perf/util/block-range.c
index 1be432657501..11c6f33524f0 100644
--- a/tools/perf/util/block-range.c
+++ b/tools/perf/util/block-range.c
@@ -71,6 +71,22 @@ static inline void rb_link_right_of_node(struct rb_node *right, struct rb_node *
 	rb_link_node(right, node, p);
 }
 
+static inline void block_range__insert_left(struct block_range *insert,
+					     struct block_range *parent)
+{
+	rb_link_left_of_node(&insert->node, &parent->node);
+	rb_insert_color(&insert->node, &block_ranges.root);
+	block_range__debug();
+}
+
+static inline void block_range__insert_right(struct block_range *insert,
+					      struct block_range *parent)
+{
+	rb_link_right_of_node(&insert->node, &parent->node);
+	rb_insert_color(&insert->node, &block_ranges.root);
+	block_range__debug();
+}
+
 /**
  * block_range__create
  * @start: branch target starting this basic block
@@ -128,9 +144,7 @@ struct block_range_iter block_range__create(u64 start, u64 end)
 				.is_branch	= 0,
 			};
 
-			rb_link_left_of_node(&head->node, &next->node);
-			rb_insert_color(&head->node, &block_ranges.root);
-			block_range__debug();
+			block_range__insert_left(head, next);
 
 			iter.start = head;
 			goto do_tail;
@@ -182,9 +196,7 @@ struct block_range_iter block_range__create(u64 start, u64 end)
 		entry->is_target	= 1;
 		entry->entry		= 0;
 
-		rb_link_left_of_node(&head->node, &entry->node);
-		rb_insert_color(&head->node, &block_ranges.root);
-		block_range__debug();
+		block_range__insert_left(head, entry);
 
 	} else if (entry->start == start)
 		entry->is_target = 1;
@@ -222,9 +234,7 @@ struct block_range_iter block_range__create(u64 start, u64 end)
 			entry->taken		= 0;
 			entry->pred		= 0;
 
-			rb_link_right_of_node(&tail->node, &entry->node);
-			rb_insert_color(&tail->node, &block_ranges.root);
-			block_range__debug();
+			block_range__insert_right(tail, entry);
 
 			iter.end = entry;
 			goto done;
@@ -260,9 +270,7 @@ struct block_range_iter block_range__create(u64 start, u64 end)
 				.is_branch	= 1,
 			};
 
-			rb_link_right_of_node(&tail->node, &entry->node);
-			rb_insert_color(&tail->node, &block_ranges.root);
-			block_range__debug();
+			block_range__insert_right(tail, entry);
 
 			iter.end = tail;
 			goto done;
@@ -283,9 +291,7 @@ struct block_range_iter block_range__create(u64 start, u64 end)
 				.is_branch	= 0,
 			};
 
-			rb_link_left_of_node(&hole->node, &next->node);
-			rb_insert_color(&hole->node, &block_ranges.root);
-			block_range__debug();
+			block_range__insert_left(hole, next);
 		}
 
 		entry = next;
-- 
2.17.1

[PATCH 3/5] perf dso: Merge cases in dso__read_binary_type_filename

[Shang XiaoJing]

Merge the cases having same procedure processing in
dso__read_binary_type_filename.

Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
 tools/perf/util/dso.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index f1a14c0ad26d..6357ac1e9015 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -184,6 +184,8 @@ int dso__read_binary_type_filename(const struct dso *dso,
 	case DSO_BINARY_TYPE__VMLINUX:
 	case DSO_BINARY_TYPE__GUEST_VMLINUX:
 	case DSO_BINARY_TYPE__SYSTEM_PATH_DSO:
+	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
+	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
 		__symbol__join_symfs(filename, size, dso->long_name);
 		break;
 
@@ -193,11 +195,6 @@ int dso__read_binary_type_filename(const struct dso *dso,
 			    root_dir, dso->long_name);
 		break;
 
-	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE:
-	case DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP:
-		__symbol__join_symfs(filename, size, dso->long_name);
-		break;
-
 	case DSO_BINARY_TYPE__KCORE:
 	case DSO_BINARY_TYPE__GUEST_KCORE:
 		snprintf(filename, size, "%s", dso->long_name);
-- 
2.17.1

[PATCH 4/5] perf stat: Add print_cache_common helper

[Shang XiaoJing]

Wrap repeated code in helper function print_cache_common, which is used
to print cache event.

Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
 tools/perf/util/stat-shadow.c | 80 +++++++++++------------------------
 1 file changed, 25 insertions(+), 55 deletions(-)

diff --git a/tools/perf/util/stat-shadow.c b/tools/perf/util/stat-shadow.c
index 788ce5e46470..706b71db50f7 100644
--- a/tools/perf/util/stat-shadow.c
+++ b/tools/perf/util/stat-shadow.c
@@ -533,23 +533,34 @@ static void print_stalled_cycles_backend(struct perf_stat_config *config,
 	out->print_metric(config, out->ctx, color, "%7.2f%%", "backend cycles idle", ratio);
 }
 
-static void print_branch_misses(struct perf_stat_config *config,
+static void print_cache_common(struct perf_stat_config *config,
 				int cpu_map_idx, double avg,
 				struct perf_stat_output_ctx *out,
 				struct runtime_stat *st,
-				struct runtime_stat_data *rsd)
+				struct runtime_stat_data *rsd,
+				enum stat_type type, const char *desc)
 {
 	double total, ratio = 0.0;
 	const char *color;
 
-	total = runtime_stat_avg(st, STAT_BRANCHES, cpu_map_idx, rsd);
+	total = runtime_stat_avg(st, type, cpu_map_idx, rsd);
 
 	if (total)
 		ratio = avg / total * 100.0;
 
 	color = get_ratio_color(GRC_CACHE_MISSES, ratio);
 
-	out->print_metric(config, out->ctx, color, "%7.2f%%", "of all branches", ratio);
+	out->print_metric(config, out->ctx, color, "%7.2f%%", desc, ratio);
+}
+
+static void print_branch_misses(struct perf_stat_config *config,
+				int cpu_map_idx, double avg,
+				struct perf_stat_output_ctx *out,
+				struct runtime_stat *st,
+				struct runtime_stat_data *rsd)
+{
+	print_cache_common(config, cpu_map_idx, avg, out, st, rsd,
+			   STAT_BRANCHES, "of all branches");
 }
 
 static void print_l1_dcache_misses(struct perf_stat_config *config,
@@ -558,17 +569,8 @@ static void print_l1_dcache_misses(struct perf_stat_config *config,
 				   struct runtime_stat *st,
 				   struct runtime_stat_data *rsd)
 {
-	double total, ratio = 0.0;
-	const char *color;
-
-	total = runtime_stat_avg(st, STAT_L1_DCACHE, cpu_map_idx, rsd);
-
-	if (total)
-		ratio = avg / total * 100.0;
-
-	color = get_ratio_color(GRC_CACHE_MISSES, ratio);
-
-	out->print_metric(config, out->ctx, color, "%7.2f%%", "of all L1-dcache accesses", ratio);
+	print_cache_common(config, cpu_map_idx, avg, out, st, rsd,
+			   STAT_L1_DCACHE, "of all L1-dcache accesses");
 }
 
 static void print_l1_icache_misses(struct perf_stat_config *config,
@@ -577,16 +579,8 @@ static void print_l1_icache_misses(struct perf_stat_config *config,
 				   struct runtime_stat *st,
 				   struct runtime_stat_data *rsd)
 {
-	double total, ratio = 0.0;
-	const char *color;
-
-	total = runtime_stat_avg(st, STAT_L1_ICACHE, cpu_map_idx, rsd);
-
-	if (total)
-		ratio = avg / total * 100.0;
-
-	color = get_ratio_color(GRC_CACHE_MISSES, ratio);
-	out->print_metric(config, out->ctx, color, "%7.2f%%", "of all L1-icache accesses", ratio);
+	print_cache_common(config, cpu_map_idx, avg, out, st, rsd,
+			   STAT_L1_ICACHE, "of all L1-icache accesses");
 }
 
 static void print_dtlb_cache_misses(struct perf_stat_config *config,
@@ -595,16 +589,8 @@ static void print_dtlb_cache_misses(struct perf_stat_config *config,
 				    struct runtime_stat *st,
 				    struct runtime_stat_data *rsd)
 {
-	double total, ratio = 0.0;
-	const char *color;
-
-	total = runtime_stat_avg(st, STAT_DTLB_CACHE, cpu_map_idx, rsd);
-
-	if (total)
-		ratio = avg / total * 100.0;
-
-	color = get_ratio_color(GRC_CACHE_MISSES, ratio);
-	out->print_metric(config, out->ctx, color, "%7.2f%%", "of all dTLB cache accesses", ratio);
+	print_cache_common(config, cpu_map_idx, avg, out, st, rsd,
+			   STAT_DTLB_CACHE, "of all dTLB cache accesses");
 }
 
 static void print_itlb_cache_misses(struct perf_stat_config *config,
@@ -613,16 +599,8 @@ static void print_itlb_cache_misses(struct perf_stat_config *config,
 				    struct runtime_stat *st,
 				    struct runtime_stat_data *rsd)
 {
-	double total, ratio = 0.0;
-	const char *color;
-
-	total = runtime_stat_avg(st, STAT_ITLB_CACHE, cpu_map_idx, rsd);
-
-	if (total)
-		ratio = avg / total * 100.0;
-
-	color = get_ratio_color(GRC_CACHE_MISSES, ratio);
-	out->print_metric(config, out->ctx, color, "%7.2f%%", "of all iTLB cache accesses", ratio);
+	print_cache_common(config, cpu_map_idx, avg, out, st, rsd,
+			   STAT_ITLB_CACHE, "of all iTLB cache accesses");
 }
 
 static void print_ll_cache_misses(struct perf_stat_config *config,
@@ -631,16 +609,8 @@ static void print_ll_cache_misses(struct perf_stat_config *config,
 				  struct runtime_stat *st,
 				  struct runtime_stat_data *rsd)
 {
-	double total, ratio = 0.0;
-	const char *color;
-
-	total = runtime_stat_avg(st, STAT_LL_CACHE, cpu_map_idx, rsd);
-
-	if (total)
-		ratio = avg / total * 100.0;
-
-	color = get_ratio_color(GRC_CACHE_MISSES, ratio);
-	out->print_metric(config, out->ctx, color, "%7.2f%%", "of all LL-cache accesses", ratio);
+	print_cache_common(config, cpu_map_idx, avg, out, st, rsd,
+			   STAT_LL_CACHE, "of all LL-cache accesses");
 }
 
 /*
-- 
2.17.1

[PATCH 5/5] perf genelf: Add create_section_and_get_data helper

[Shang XiaoJing]

Wrap repeated code in helper function create_section_and_get_data, which
create section and new data by using Elf.

Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
 tools/perf/util/genelf.c | 96 +++++++++++-----------------------------
 1 file changed, 25 insertions(+), 71 deletions(-)

diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
index 953338b9e887..0aa60139b9ca 100644
--- a/tools/perf/util/genelf.c
+++ b/tools/perf/util/genelf.c
@@ -155,17 +155,8 @@ gen_build_id(struct buildid_note *note, unsigned long load_addr, const void *cod
 #endif
 
 static int
-jit_add_eh_frame_info(Elf *e, void* unwinding, uint64_t unwinding_header_size,
-		      uint64_t unwinding_size, uint64_t base_offset)
+create_section_and_get_data(Elf *e, Elf_Scn *scn, Elf_Data *d)
 {
-	Elf_Data *d;
-	Elf_Scn *scn;
-	Elf_Shdr *shdr;
-	uint64_t unwinding_table_size = unwinding_size - unwinding_header_size;
-
-	/*
-	 * setup eh_frame section
-	 */
 	scn = elf_newscn(e);
 	if (!scn) {
 		warnx("cannot create section");
@@ -177,6 +168,23 @@ jit_add_eh_frame_info(Elf *e, void* unwinding, uint64_t unwinding_header_size,
 		warnx("cannot get new data");
 		return -1;
 	}
+	return 0;
+}
+
+static int
+jit_add_eh_frame_info(Elf *e, void* unwinding, uint64_t unwinding_header_size,
+		      uint64_t unwinding_size, uint64_t base_offset)
+{
+	Elf_Data *d = NULL;
+	Elf_Scn *scn = NULL;
+	Elf_Shdr *shdr;
+	uint64_t unwinding_table_size = unwinding_size - unwinding_header_size;
+
+	/*
+	 * setup eh_frame section
+	 */
+	if (create_section_and_get_data(e, scn, d))
+		return -1;
 
 	d->d_align = 8;
 	d->d_off = 0LL;
@@ -200,17 +208,8 @@ jit_add_eh_frame_info(Elf *e, void* unwinding, uint64_t unwinding_header_size,
 	/*
 	 * setup eh_frame_hdr section
 	 */
-	scn = elf_newscn(e);
-	if (!scn) {
-		warnx("cannot create section");
+	if (create_section_and_get_data(e, scn, d))
 		return -1;
-	}
-
-	d = elf_newdata(scn);
-	if (!d) {
-		warnx("cannot get new data");
-		return -1;
-	}
 
 	d->d_align = 4;
 	d->d_off = 0LL;
@@ -248,7 +247,7 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
 	      void *unwinding, uint64_t unwinding_header_size, uint64_t unwinding_size)
 {
 	Elf *e;
-	Elf_Data *d;
+	Elf_Data *d = NULL;
 	Elf_Scn *scn;
 	Elf_Ehdr *ehdr;
 	Elf_Shdr *shdr;
@@ -288,17 +287,8 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
 	/*
 	 * setup text section
 	 */
-	scn = elf_newscn(e);
-	if (!scn) {
-		warnx("cannot create section");
+	if (create_section_and_get_data(e, scn, d))
 		goto error;
-	}
-
-	d = elf_newdata(scn);
-	if (!d) {
-		warnx("cannot get new data");
-		goto error;
-	}
 
 	d->d_align = 16;
 	d->d_off = 0LL;
@@ -334,17 +324,8 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
 	/*
 	 * setup section headers string table
 	 */
-	scn = elf_newscn(e);
-	if (!scn) {
-		warnx("cannot create section");
+	if (create_section_and_get_data(e, scn, d))
 		goto error;
-	}
-
-	d = elf_newdata(scn);
-	if (!d) {
-		warnx("cannot get new data");
-		goto error;
-	}
 
 	d->d_align = 1;
 	d->d_off = 0LL;
@@ -370,17 +351,8 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
 	symtab[1].st_size  = csize;
 	symtab[1].st_value = GEN_ELF_TEXT_OFFSET;
 
-	scn = elf_newscn(e);
-	if (!scn) {
-		warnx("cannot create section");
-		goto error;
-	}
-
-	d = elf_newdata(scn);
-	if (!d) {
-		warnx("cannot get new data");
+	if (create_section_and_get_data(e, scn, d))
 		goto error;
-	}
 
 	d->d_align = 8;
 	d->d_off = 0LL;
@@ -413,17 +385,8 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
 	}
 	strcpy(strsym + 1, sym);
 
-	scn = elf_newscn(e);
-	if (!scn) {
-		warnx("cannot create section");
-		goto error;
-	}
-
-	d = elf_newdata(scn);
-	if (!d) {
-		warnx("cannot get new data");
+	if (create_section_and_get_data(e, scn, d))
 		goto error;
-	}
 
 	d->d_align = 1;
 	d->d_off = 0LL;
@@ -446,17 +409,8 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
 	/*
 	 * setup build-id section
 	 */
-	scn = elf_newscn(e);
-	if (!scn) {
-		warnx("cannot create section");
-		goto error;
-	}
-
-	d = elf_newdata(scn);
-	if (!d) {
-		warnx("cannot get new data");
+	if (create_section_and_get_data(e, scn, d))
 		goto error;
-	}
 
 	/*
 	 * build-id generation
-- 
2.17.1