[PATCH v2 0/7] perf kwork: A couple of fixes according to review comments

[PATCH v2 0/7] perf kwork: A couple of fixes according to review comments

[Yang Jihong]

Some issues are fixed based on the comments of several reviewers:
1. Add '--synth task' option for record (note from Namhyung)
2. Replace hard-coded initialization of nr_tracepoints with ARRAY_SIZE macro (note from Namhyung)
3. Fix spelling mistake: "PRINT_TIME_UNIT_MESC_WIDTH" -> "PRINT_TIME_UNIT_MSEC_WIDTH" (note from Namhyung)
4. Fix spelling mistake: "Captuer" -> "Capture" (note from Colin)
5. Add some {} for multiline for/if blocks (note from Arnaldo)

Modifying based on the latest commit (9a0b36266f7a) at acme/perf/core:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/core

Yang Jihong (7):
  perf kwork: Add '--synth task' option for record
  perf kwork: Replace hard-coded initialization of nr_tracepoints with
    ARRAY_SIZE macro
  perf kwork: Fix spelling mistake: "Captuer" -> "Capture"
  perf kwork: Fix spelling mistake: "PRINT_TIME_UNIT_MESC_WIDTH" ->
    "PRINT_TIME_UNIT_MSEC_WIDTH"
  perf kwork: Strdup function name in workqueue_work_init
  perf kwork: Expand capacity of bpf entries
  perf kwork: Add some {} for multiline for/if blocks

 tools/perf/builtin-kwork.c                 | 67 +++++++++++++---------
 tools/perf/util/bpf_skel/kwork_trace.bpf.c |  2 +-
 2 files changed, 40 insertions(+), 29 deletions(-)

-- 
2.30.GIT

[PATCH v2 1/7] perf kwork: Add '--synth task' option for record

[Yang Jihong]

Since we don't need user space symbols, add '--synth task' option to skip
costly synthesis.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
 tools/perf/builtin-kwork.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index fb8c63656ad8..ff17711a554a 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -1631,6 +1631,10 @@ static int perf_kwork__record(struct perf_kwork *kwork,
 		"-R",
 		"-m", "1024",
 		"-c", "1",
+		/*
+		 * Skip synthesis as we do not need user space symbols.
+		 */
+		"--synth", "task",
 	};
 
 	rec_argc = ARRAY_SIZE(record_args) + argc - 1;
-- 
2.30.GIT

[PATCH v2 2/7] perf kwork: Replace hard-coded initialization of nr_tracepoints with ARRAY_SIZE macro

[Yang Jihong]

Use ARRAY_SIZE macro to replace hard-coded initialization of nr_tracepoints
for future changes.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
 tools/perf/builtin-kwork.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index ff17711a554a..2143280c2e95 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -817,7 +817,7 @@ static void irq_work_name(struct kwork_work *work, char *buf, int len)
 static struct kwork_class kwork_irq = {
 	.name           = "irq",
 	.type           = KWORK_CLASS_IRQ,
-	.nr_tracepoints = 2,
+	.nr_tracepoints = ARRAY_SIZE(irq_tp_handlers),
 	.tp_handlers    = irq_tp_handlers,
 	.class_init     = irq_class_init,
 	.work_init      = irq_work_init,
@@ -938,7 +938,7 @@ static void softirq_work_name(struct kwork_work *work, char *buf, int len)
 static struct kwork_class kwork_softirq = {
 	.name           = "softirq",
 	.type           = KWORK_CLASS_SOFTIRQ,
-	.nr_tracepoints = 3,
+	.nr_tracepoints = ARRAY_SIZE(softirq_tp_handlers),
 	.tp_handlers    = softirq_tp_handlers,
 	.class_init     = softirq_class_init,
 	.work_init      = softirq_work_init,
@@ -1035,7 +1035,7 @@ static void workqueue_work_name(struct kwork_work *work, char *buf, int len)
 static struct kwork_class kwork_workqueue = {
 	.name           = "workqueue",
 	.type           = KWORK_CLASS_WORKQUEUE,
-	.nr_tracepoints = 3,
+	.nr_tracepoints = ARRAY_SIZE(workqueue_tp_handlers),
 	.tp_handlers    = workqueue_tp_handlers,
 	.class_init     = workqueue_class_init,
 	.work_init      = workqueue_work_init,
-- 
2.30.GIT

[PATCH v2 3/7] perf kwork: Fix spelling mistake: "Captuer" -> "Capture"

[Yang Jihong]

Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
 tools/perf/builtin-kwork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index 2143280c2e95..9e8b9f9e0472 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -1447,7 +1447,7 @@ static void sig_handler(int sig)
 	 * Simply capture termination signal so that
 	 * the program can continue after pause returns
 	 */
-	pr_debug("Captuer signal %d\n", sig);
+	pr_debug("Capture signal %d\n", sig);
 }
 
 static int perf_kwork__report_bpf(struct perf_kwork *kwork)
-- 
2.30.GIT

[PATCH v2 4/7] perf kwork: Fix spelling mistake: "PRINT_TIME_UNIT_MESC_WIDTH" -> "PRINT_TIME_UNIT_MSEC_WIDTH"

[Yang Jihong]

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
 tools/perf/builtin-kwork.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index 9e8b9f9e0472..d6c09044a929 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -37,9 +37,9 @@
 #define RPINT_DECIMAL_WIDTH 3
 #define PRINT_BRACKETPAIR_WIDTH 2
 #define PRINT_TIME_UNIT_SEC_WIDTH 2
-#define PRINT_TIME_UNIT_MESC_WIDTH 3
-#define PRINT_RUNTIME_HEADER_WIDTH (PRINT_RUNTIME_WIDTH + PRINT_TIME_UNIT_MESC_WIDTH)
-#define PRINT_LATENCY_HEADER_WIDTH (PRINT_LATENCY_WIDTH + PRINT_TIME_UNIT_MESC_WIDTH)
+#define PRINT_TIME_UNIT_MSEC_WIDTH 3
+#define PRINT_RUNTIME_HEADER_WIDTH (PRINT_RUNTIME_WIDTH + PRINT_TIME_UNIT_MSEC_WIDTH)
+#define PRINT_LATENCY_HEADER_WIDTH (PRINT_LATENCY_WIDTH + PRINT_TIME_UNIT_MSEC_WIDTH)
 #define PRINT_TIMEHIST_CPU_WIDTH (PRINT_CPU_WIDTH + PRINT_BRACKETPAIR_WIDTH)
 #define PRINT_TIMESTAMP_HEADER_WIDTH (PRINT_TIMESTAMP_WIDTH + PRINT_TIME_UNIT_SEC_WIDTH)
 
-- 
2.30.GIT

[PATCH v2 5/7] perf kwork: Strdup function name in workqueue_work_init

[Yang Jihong]

perf_session has been deleted when report, maps data in session has been free,
we need to copy duplicate of workqueue function symbol for future use.

Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
 tools/perf/builtin-kwork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index d6c09044a929..33bcab2aafcd 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -1021,7 +1021,7 @@ static void workqueue_work_init(struct kwork_class *class,
 	work->cpu = sample->cpu;
 	work->id = evsel__intval(evsel, sample, "work");
 	work->name = function_addr == 0 ? NULL :
-		machine__resolve_kernel_addr(machine, &function_addr, &modp);
+		strdup(machine__resolve_kernel_addr(machine, &function_addr, &modp));
 }
 
 static void workqueue_work_name(struct kwork_work *work, char *buf, int len)
-- 
2.30.GIT

[PATCH v2 6/7] perf kwork: Expand capacity of bpf entries

[Yang Jihong]

100 kwork entries are not enough for a system with hundreds of cores,
improving capacity.

Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
 tools/perf/util/bpf_skel/kwork_trace.bpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/bpf_skel/kwork_trace.bpf.c b/tools/perf/util/bpf_skel/kwork_trace.bpf.c
index 063c124e0999..5300e8be64b0 100644
--- a/tools/perf/util/bpf_skel/kwork_trace.bpf.c
+++ b/tools/perf/util/bpf_skel/kwork_trace.bpf.c
@@ -5,7 +5,7 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
 
-#define KWORK_COUNT 100
+#define KWORK_COUNT 10000
 #define MAX_KWORKNAME 128
 
 /*
-- 
2.30.GIT

[PATCH v2 7/7] perf kwork: Add some {} for multiline for/if blocks

[Yang Jihong]

Suggested-by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
 tools/perf/builtin-kwork.c | 47 ++++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/tools/perf/builtin-kwork.c b/tools/perf/builtin-kwork.c
index 33bcab2aafcd..f33666ca3ee6 100644
--- a/tools/perf/builtin-kwork.c
+++ b/tools/perf/builtin-kwork.c
@@ -177,9 +177,10 @@ static void setup_sorting(struct perf_kwork *kwork,
 
 	for (tok = strtok_r(str, ", ", &tmp);
 	     tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		if (sort_dimension__add(kwork, tok, &kwork->sort_list) < 0)
+		if (sort_dimension__add(kwork, tok, &kwork->sort_list) < 0) {
 			usage_with_options_msg(usage_msg, options,
 					       "Unknown --sort key: `%s'", tok);
+		}
 	}
 
 	pr_debug("Sort order: %s\n", kwork->sort_order);
@@ -656,8 +657,9 @@ static void timehist_print_event(struct perf_kwork *kwork,
 		work->class->work_name(work, kwork_name,
 				       PRINT_KWORK_NAME_WIDTH);
 		printf(" %-*s ", PRINT_KWORK_NAME_WIDTH, kwork_name);
-	} else
+	} else {
 		printf(" %-*s ", PRINT_KWORK_NAME_WIDTH, "");
+	}
 
 	/*
 	 *runtime
@@ -669,11 +671,12 @@ static void timehist_print_event(struct perf_kwork *kwork,
 	/*
 	 * delaytime
 	 */
-	if (atom->prev != NULL)
+	if (atom->prev != NULL) {
 		printf(" %*.*f ", PRINT_LATENCY_WIDTH, RPINT_DECIMAL_WIDTH,
 		       (double)(atom->time - atom->prev->time) / NSEC_PER_MSEC);
-	else
+	} else {
 		printf(" %*s ", PRINT_LATENCY_WIDTH, " ");
+	}
 
 	/*
 	 * callchain
@@ -761,9 +764,10 @@ static int process_irq_handler_entry_event(struct perf_tool *tool,
 {
 	struct perf_kwork *kwork = container_of(tool, struct perf_kwork, tool);
 
-	if (kwork->tp_handler->entry_event)
+	if (kwork->tp_handler->entry_event) {
 		return kwork->tp_handler->entry_event(kwork, &kwork_irq,
 						      evsel, sample, machine);
+	}
 	return 0;
 }
 
@@ -774,9 +778,10 @@ static int process_irq_handler_exit_event(struct perf_tool *tool,
 {
 	struct perf_kwork *kwork = container_of(tool, struct perf_kwork, tool);
 
-	if (kwork->tp_handler->exit_event)
+	if (kwork->tp_handler->exit_event) {
 		return kwork->tp_handler->exit_event(kwork, &kwork_irq,
 						     evsel, sample, machine);
+	}
 	return 0;
 }
 
@@ -832,10 +837,10 @@ static int process_softirq_raise_event(struct perf_tool *tool,
 {
 	struct perf_kwork *kwork = container_of(tool, struct perf_kwork, tool);
 
-	if (kwork->tp_handler->raise_event)
+	if (kwork->tp_handler->raise_event) {
 		return kwork->tp_handler->raise_event(kwork, &kwork_softirq,
 						      evsel, sample, machine);
-
+	}
 	return 0;
 }
 
@@ -846,10 +851,10 @@ static int process_softirq_entry_event(struct perf_tool *tool,
 {
 	struct perf_kwork *kwork = container_of(tool, struct perf_kwork, tool);
 
-	if (kwork->tp_handler->entry_event)
+	if (kwork->tp_handler->entry_event) {
 		return kwork->tp_handler->entry_event(kwork, &kwork_softirq,
 						      evsel, sample, machine);
-
+	}
 	return 0;
 }
 
@@ -860,10 +865,10 @@ static int process_softirq_exit_event(struct perf_tool *tool,
 {
 	struct perf_kwork *kwork = container_of(tool, struct perf_kwork, tool);
 
-	if (kwork->tp_handler->exit_event)
+	if (kwork->tp_handler->exit_event) {
 		return kwork->tp_handler->exit_event(kwork, &kwork_softirq,
 						     evsel, sample, machine);
-
+	}
 	return 0;
 }
 
@@ -953,10 +958,10 @@ static int process_workqueue_activate_work_event(struct perf_tool *tool,
 {
 	struct perf_kwork *kwork = container_of(tool, struct perf_kwork, tool);
 
-	if (kwork->tp_handler->raise_event)
+	if (kwork->tp_handler->raise_event) {
 		return kwork->tp_handler->raise_event(kwork, &kwork_workqueue,
 						    evsel, sample, machine);
-
+	}
 	return 0;
 }
 
@@ -967,10 +972,10 @@ static int process_workqueue_execute_start_event(struct perf_tool *tool,
 {
 	struct perf_kwork *kwork = container_of(tool, struct perf_kwork, tool);
 
-	if (kwork->tp_handler->entry_event)
+	if (kwork->tp_handler->entry_event) {
 		return kwork->tp_handler->entry_event(kwork, &kwork_workqueue,
 						    evsel, sample, machine);
-
+	}
 	return 0;
 }
 
@@ -981,10 +986,10 @@ static int process_workqueue_execute_end_event(struct perf_tool *tool,
 {
 	struct perf_kwork *kwork = container_of(tool, struct perf_kwork, tool);
 
-	if (kwork->tp_handler->exit_event)
+	if (kwork->tp_handler->exit_event) {
 		return kwork->tp_handler->exit_event(kwork, &kwork_workqueue,
 						   evsel, sample, machine);
-
+	}
 	return 0;
 }
 
@@ -1253,9 +1258,10 @@ static void print_skipped_events(struct perf_kwork *kwork)
 		}
 	}
 
-	if (verbose > 0)
+	if (verbose > 0) {
 		printf("  INFO: use %lld atom pages\n",
 		       nr_list_entry(&kwork->atom_page_list));
+	}
 }
 
 static void print_bad_events(struct perf_kwork *kwork)
@@ -1332,10 +1338,11 @@ static int perf_kwork__check_config(struct perf_kwork *kwork,
 		return -1;
 	}
 
-	list_for_each_entry(class, &kwork->class_list, list)
+	list_for_each_entry(class, &kwork->class_list, list) {
 		if ((class->class_init != NULL) &&
 		    (class->class_init(class, session) != 0))
 			return -1;
+	}
 
 	if (kwork->cpu_list != NULL) {
 		ret = perf_session__cpu_bitmap(session,
-- 
2.30.GIT

Re: [PATCH v2 0/7] perf kwork: A couple of fixes according to review comments

[Yang Jihong]

Hello,

PING.

Regards,
Jihong
.

On 2022/7/30 11:03, Yang Jihong wrote:
> Some issues are fixed based on the comments of several reviewers:
> 1. Add '--synth task' option for record (note from Namhyung)
> 2. Replace hard-coded initialization of nr_tracepoints with ARRAY_SIZE macro (note from Namhyung)
> 3. Fix spelling mistake: "PRINT_TIME_UNIT_MESC_WIDTH" -> "PRINT_TIME_UNIT_MSEC_WIDTH" (note from Namhyung)
> 4. Fix spelling mistake: "Captuer" -> "Capture" (note from Colin)
> 5. Add some {} for multiline for/if blocks (note from Arnaldo)
> 
> Modifying based on the latest commit (9a0b36266f7a) at acme/perf/core:
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/core
> 
> Yang Jihong (7):
>    perf kwork: Add '--synth task' option for record
>    perf kwork: Replace hard-coded initialization of nr_tracepoints with
>      ARRAY_SIZE macro
>    perf kwork: Fix spelling mistake: "Captuer" -> "Capture"
>    perf kwork: Fix spelling mistake: "PRINT_TIME_UNIT_MESC_WIDTH" ->
>      "PRINT_TIME_UNIT_MSEC_WIDTH"
>    perf kwork: Strdup function name in workqueue_work_init
>    perf kwork: Expand capacity of bpf entries
>    perf kwork: Add some {} for multiline for/if blocks
> 
>   tools/perf/builtin-kwork.c                 | 67 +++++++++++++---------
>   tools/perf/util/bpf_skel/kwork_trace.bpf.c |  2 +-
>   2 files changed, 40 insertions(+), 29 deletions(-)
>