* [PATCH 1/4] perf trace: Use zalloc to save initialization of syscall_stats
2022-09-08 2:11 [PATCH 0/4] perf: Clean up by adding helpers Shang XiaoJing
@ 2022-09-08 2:11 ` Shang XiaoJing
2022-09-08 2:11 ` [PATCH 2/4] perf lock: Add get_key_by_aggr_mode helper Shang XiaoJing
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Shang XiaoJing @ 2022-09-08 2:11 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, linux-perf-users, linux-kernel
Cc: shangxiaojing
As most members of syscall_stats is set to 0 in thread__update_stats,
using zalloc directly.
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
tools/perf/builtin-trace.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 0bd9d01c0df9..3ecc31375f90 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2173,13 +2173,10 @@ static void thread__update_stats(struct thread *thread, struct thread_trace *ttr
stats = inode->priv;
if (stats == NULL) {
- stats = malloc(sizeof(*stats));
+ stats = zalloc(sizeof(*stats));
if (stats == NULL)
return;
- stats->nr_failures = 0;
- stats->max_errno = 0;
- stats->errnos = NULL;
init_stats(&stats->stats);
inode->priv = stats;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/4] perf lock: Add get_key_by_aggr_mode helper
2022-09-08 2:11 [PATCH 0/4] perf: Clean up by adding helpers Shang XiaoJing
2022-09-08 2:11 ` [PATCH 1/4] perf trace: Use zalloc to save initialization of syscall_stats Shang XiaoJing
@ 2022-09-08 2:11 ` Shang XiaoJing
2022-09-08 2:11 ` [PATCH 3/4] perf timechart: Add create_pidcomm helper Shang XiaoJing
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Shang XiaoJing @ 2022-09-08 2:11 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, linux-perf-users, linux-kernel
Cc: shangxiaojing
Wrap repeated code in helper functions get_key_by_aggr_mode and
get_key_by_aggr_mode_simple, which assign the value to key based on
aggregation mode. Note that for the conditions not support
LOCK_AGGR_CALLER, should call get_key_by_aggr_mode_simple directly.
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
tools/perf/builtin-lock.c | 129 ++++++++++++++++----------------------
1 file changed, 53 insertions(+), 76 deletions(-)
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index 70197c0593b1..44a47648b7fe 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -560,29 +560,50 @@ enum acquire_flags {
READ_LOCK = 2,
};
-static int report_lock_acquire_event(struct evsel *evsel,
- struct perf_sample *sample)
+static int get_key_by_aggr_mode_simple(u64 *key, u64 addr, u32 tid)
{
- struct lock_stat *ls;
- struct thread_stat *ts;
- struct lock_seq_stat *seq;
- const char *name = evsel__strval(evsel, sample, "name");
- u64 addr = evsel__intval(evsel, sample, "lockdep_addr");
- int flag = evsel__intval(evsel, sample, "flags");
- u64 key;
-
switch (aggr_mode) {
case LOCK_AGGR_ADDR:
- key = addr;
+ *key = addr;
break;
case LOCK_AGGR_TASK:
- key = sample->tid;
+ *key = tid;
break;
case LOCK_AGGR_CALLER:
default:
pr_err("Invalid aggregation mode: %d\n", aggr_mode);
return -EINVAL;
}
+ return 0;
+}
+
+static u64 callchain_id(struct evsel *evsel, struct perf_sample *sample);
+
+static int get_key_by_aggr_mode(u64 *key, u64 addr, struct evsel *evsel,
+ struct perf_sample *sample)
+{
+ if (aggr_mode == LOCK_AGGR_CALLER) {
+ *key = callchain_id(evsel, sample);
+ return 0;
+ }
+ return get_key_by_aggr_mode_simple(key, addr, sample->tid);
+}
+
+static int report_lock_acquire_event(struct evsel *evsel,
+ struct perf_sample *sample)
+{
+ struct lock_stat *ls;
+ struct thread_stat *ts;
+ struct lock_seq_stat *seq;
+ const char *name = evsel__strval(evsel, sample, "name");
+ u64 addr = evsel__intval(evsel, sample, "lockdep_addr");
+ int flag = evsel__intval(evsel, sample, "flags");
+ u64 key;
+ int ret;
+
+ ret = get_key_by_aggr_mode_simple(&key, addr, sample->tid);
+ if (ret < 0)
+ return ret;
ls = lock_stat_findnew(key, name, 0);
if (!ls)
@@ -653,19 +674,11 @@ static int report_lock_acquired_event(struct evsel *evsel,
const char *name = evsel__strval(evsel, sample, "name");
u64 addr = evsel__intval(evsel, sample, "lockdep_addr");
u64 key;
+ int ret;
- switch (aggr_mode) {
- case LOCK_AGGR_ADDR:
- key = addr;
- break;
- case LOCK_AGGR_TASK:
- key = sample->tid;
- break;
- case LOCK_AGGR_CALLER:
- default:
- pr_err("Invalid aggregation mode: %d\n", aggr_mode);
- return -EINVAL;
- }
+ ret = get_key_by_aggr_mode_simple(&key, addr, sample->tid);
+ if (ret < 0)
+ return ret;
ls = lock_stat_findnew(key, name, 0);
if (!ls)
@@ -726,19 +739,11 @@ static int report_lock_contended_event(struct evsel *evsel,
const char *name = evsel__strval(evsel, sample, "name");
u64 addr = evsel__intval(evsel, sample, "lockdep_addr");
u64 key;
+ int ret;
- switch (aggr_mode) {
- case LOCK_AGGR_ADDR:
- key = addr;
- break;
- case LOCK_AGGR_TASK:
- key = sample->tid;
- break;
- case LOCK_AGGR_CALLER:
- default:
- pr_err("Invalid aggregation mode: %d\n", aggr_mode);
- return -EINVAL;
- }
+ ret = get_key_by_aggr_mode_simple(&key, addr, sample->tid);
+ if (ret < 0)
+ return ret;
ls = lock_stat_findnew(key, name, 0);
if (!ls)
@@ -792,19 +797,11 @@ static int report_lock_release_event(struct evsel *evsel,
const char *name = evsel__strval(evsel, sample, "name");
u64 addr = evsel__intval(evsel, sample, "lockdep_addr");
u64 key;
+ int ret;
- switch (aggr_mode) {
- case LOCK_AGGR_ADDR:
- key = addr;
- break;
- case LOCK_AGGR_TASK:
- key = sample->tid;
- break;
- case LOCK_AGGR_CALLER:
- default:
- pr_err("Invalid aggregation mode: %d\n", aggr_mode);
- return -EINVAL;
- }
+ ret = get_key_by_aggr_mode_simple(&key, addr, sample->tid);
+ if (ret < 0)
+ return ret;
ls = lock_stat_findnew(key, name, 0);
if (!ls)
@@ -1015,21 +1012,11 @@ static int report_lock_contention_begin_event(struct evsel *evsel,
struct lock_seq_stat *seq;
u64 addr = evsel__intval(evsel, sample, "lock_addr");
u64 key;
+ int ret;
- switch (aggr_mode) {
- case LOCK_AGGR_ADDR:
- key = addr;
- break;
- case LOCK_AGGR_TASK:
- key = sample->tid;
- break;
- case LOCK_AGGR_CALLER:
- key = callchain_id(evsel, sample);
- break;
- default:
- pr_err("Invalid aggregation mode: %d\n", aggr_mode);
- return -EINVAL;
- }
+ ret = get_key_by_aggr_mode(&key, addr, evsel, sample);
+ if (ret < 0)
+ return ret;
ls = lock_stat_find(key);
if (!ls) {
@@ -1098,21 +1085,11 @@ static int report_lock_contention_end_event(struct evsel *evsel,
u64 contended_term;
u64 addr = evsel__intval(evsel, sample, "lock_addr");
u64 key;
+ int ret;
- switch (aggr_mode) {
- case LOCK_AGGR_ADDR:
- key = addr;
- break;
- case LOCK_AGGR_TASK:
- key = sample->tid;
- break;
- case LOCK_AGGR_CALLER:
- key = callchain_id(evsel, sample);
- break;
- default:
- pr_err("Invalid aggregation mode: %d\n", aggr_mode);
- return -EINVAL;
- }
+ ret = get_key_by_aggr_mode(&key, addr, evsel, sample);
+ if (ret < 0)
+ return ret;
ls = lock_stat_find(key);
if (!ls)
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/4] perf timechart: Add create_pidcomm helper
2022-09-08 2:11 [PATCH 0/4] perf: Clean up by adding helpers Shang XiaoJing
2022-09-08 2:11 ` [PATCH 1/4] perf trace: Use zalloc to save initialization of syscall_stats Shang XiaoJing
2022-09-08 2:11 ` [PATCH 2/4] perf lock: Add get_key_by_aggr_mode helper Shang XiaoJing
@ 2022-09-08 2:11 ` Shang XiaoJing
2022-09-08 2:11 ` [PATCH 4/4] perf timechart: Add p_state_end helper Shang XiaoJing
2022-09-08 7:08 ` [PATCH 0/4] perf: Clean up by adding helpers Namhyung Kim
4 siblings, 0 replies; 7+ messages in thread
From: Shang XiaoJing @ 2022-09-08 2:11 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, linux-perf-users, linux-kernel
Cc: shangxiaojing
Wrap repeated code combined with alloc of per_pidcomm in helper function
create_pidcomm.
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
tools/perf/builtin-timechart.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index e2e9ad929baf..667a94d45493 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -215,6 +215,19 @@ static struct per_pid *find_create_pid(struct timechart *tchart, int pid)
return cursor;
}
+static struct per_pidcomm *create_pidcomm(struct per_pid *p)
+{
+ struct per_pidcomm *c;
+
+ c = zalloc(sizeof(*c));
+ if (!c)
+ return NULL;
+ p->current = c;
+ c->next = p->all;
+ p->all = c;
+ return c;
+}
+
static void pid_set_comm(struct timechart *tchart, int pid, char *comm)
{
struct per_pid *p;
@@ -233,12 +246,9 @@ static void pid_set_comm(struct timechart *tchart, int pid, char *comm)
}
c = c->next;
}
- c = zalloc(sizeof(*c));
+ c = create_pidcomm(p);
assert(c != NULL);
c->comm = strdup(comm);
- p->current = c;
- c->next = p->all;
- p->all = c;
}
static void pid_fork(struct timechart *tchart, int pid, int ppid, u64 timestamp)
@@ -277,11 +287,8 @@ static void pid_put_sample(struct timechart *tchart, int pid, int type,
p = find_create_pid(tchart, pid);
c = p->current;
if (!c) {
- c = zalloc(sizeof(*c));
+ c = create_pidcomm(p);
assert(c != NULL);
- p->current = c;
- c->next = p->all;
- p->all = c;
}
sample = zalloc(sizeof(*sample));
@@ -726,12 +733,9 @@ static int pid_begin_io_sample(struct timechart *tchart, int pid, int type,
struct io_sample *prev;
if (!c) {
- c = zalloc(sizeof(*c));
+ c = create_pidcomm(p);
if (!c)
return -ENOMEM;
- p->current = c;
- c->next = p->all;
- p->all = c;
}
prev = c->io_samples;
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 4/4] perf timechart: Add p_state_end helper
2022-09-08 2:11 [PATCH 0/4] perf: Clean up by adding helpers Shang XiaoJing
` (2 preceding siblings ...)
2022-09-08 2:11 ` [PATCH 3/4] perf timechart: Add create_pidcomm helper Shang XiaoJing
@ 2022-09-08 2:11 ` Shang XiaoJing
2022-09-08 7:08 ` [PATCH 0/4] perf: Clean up by adding helpers Namhyung Kim
4 siblings, 0 replies; 7+ messages in thread
From: Shang XiaoJing @ 2022-09-08 2:11 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, linux-perf-users, linux-kernel
Cc: shangxiaojing
Wrap repeated code in helper functions p_state_end, which alloc a new
power_event recording last pstate, and insert to the head of
tchart->power_events.
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
tools/perf/builtin-timechart.c | 37 +++++++++++++++++-----------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
index 667a94d45493..c36296bb7637 100644
--- a/tools/perf/builtin-timechart.c
+++ b/tools/perf/builtin-timechart.c
@@ -376,16 +376,13 @@ static void c_state_end(struct timechart *tchart, int cpu, u64 timestamp)
tchart->power_events = pwr;
}
-static void p_state_change(struct timechart *tchart, int cpu, u64 timestamp, u64 new_freq)
+static struct power_event *p_state_end(struct timechart *tchart, int cpu,
+ u64 timestamp)
{
- struct power_event *pwr;
-
- if (new_freq > 8000000) /* detect invalid data */
- return;
+ struct power_event *pwr = zalloc(sizeof(*pwr));
- pwr = zalloc(sizeof(*pwr));
if (!pwr)
- return;
+ return NULL;
pwr->state = cpus_pstate_state[cpu];
pwr->start_time = cpus_pstate_start_times[cpu];
@@ -393,11 +390,23 @@ static void p_state_change(struct timechart *tchart, int cpu, u64 timestamp, u64
pwr->cpu = cpu;
pwr->type = PSTATE;
pwr->next = tchart->power_events;
-
if (!pwr->start_time)
pwr->start_time = tchart->first_time;
tchart->power_events = pwr;
+ return pwr;
+}
+
+static void p_state_change(struct timechart *tchart, int cpu, u64 timestamp, u64 new_freq)
+{
+ struct power_event *pwr;
+
+ if (new_freq > 8000000) /* detect invalid data */
+ return;
+
+ pwr = p_state_end(tchart, cpu, timestamp);
+ if (!pwr)
+ return;
cpus_pstate_state[cpu] = new_freq;
cpus_pstate_start_times[cpu] = timestamp;
@@ -705,22 +714,12 @@ static void end_sample_processing(struct timechart *tchart)
#endif
/* P state */
- pwr = zalloc(sizeof(*pwr));
+ pwr = p_state_end(tchart, cpu, tchart->last_time);
if (!pwr)
return;
- pwr->state = cpus_pstate_state[cpu];
- pwr->start_time = cpus_pstate_start_times[cpu];
- pwr->end_time = tchart->last_time;
- pwr->cpu = cpu;
- pwr->type = PSTATE;
- pwr->next = tchart->power_events;
-
- if (!pwr->start_time)
- pwr->start_time = tchart->first_time;
if (!pwr->state)
pwr->state = tchart->min_freq;
- tchart->power_events = pwr;
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/4] perf: Clean up by adding helpers
2022-09-08 2:11 [PATCH 0/4] perf: Clean up by adding helpers Shang XiaoJing
` (3 preceding siblings ...)
2022-09-08 2:11 ` [PATCH 4/4] perf timechart: Add p_state_end helper Shang XiaoJing
@ 2022-09-08 7:08 ` Namhyung Kim
2022-09-08 18:23 ` Arnaldo Carvalho de Melo
4 siblings, 1 reply; 7+ messages in thread
From: Namhyung Kim @ 2022-09-08 7:08 UTC (permalink / raw)
To: Shang XiaoJing
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, linux-perf-users,
linux-kernel
Hello,
On Wed, Sep 7, 2022 at 6:37 PM Shang XiaoJing <shangxiaojing@huawei.com> wrote:
>
> Some clean up in builtin-lock.c, builtin-timechart.c, and
> builtin-trace.c.
>
> Shang XiaoJing (4):
> perf trace: Use zalloc to save initialization of syscall_stats
> perf lock: Add get_key_by_aggr_mode helper
> perf timechart: Add create_pidcomm helper
> perf timechart: Add p_state_end helper
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
>
> tools/perf/builtin-lock.c | 129 ++++++++++++++-------------------
> tools/perf/builtin-timechart.c | 65 +++++++++--------
> tools/perf/builtin-trace.c | 5 +-
> 3 files changed, 88 insertions(+), 111 deletions(-)
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 0/4] perf: Clean up by adding helpers
2022-09-08 7:08 ` [PATCH 0/4] perf: Clean up by adding helpers Namhyung Kim
@ 2022-09-08 18:23 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-09-08 18:23 UTC (permalink / raw)
To: Namhyung Kim
Cc: Shang XiaoJing, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Jiri Olsa, linux-perf-users, linux-kernel
Em Thu, Sep 08, 2022 at 12:08:06AM -0700, Namhyung Kim escreveu:
> Hello,
>
> On Wed, Sep 7, 2022 at 6:37 PM Shang XiaoJing <shangxiaojing@huawei.com> wrote:
> >
> > Some clean up in builtin-lock.c, builtin-timechart.c, and
> > builtin-trace.c.
> >
> > Shang XiaoJing (4):
> > perf trace: Use zalloc to save initialization of syscall_stats
> > perf lock: Add get_key_by_aggr_mode helper
> > perf timechart: Add create_pidcomm helper
> > perf timechart: Add p_state_end helper
>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks, applied.
- Arnaldo
> Thanks,
> Namhyung
>
>
> >
> > tools/perf/builtin-lock.c | 129 ++++++++++++++-------------------
> > tools/perf/builtin-timechart.c | 65 +++++++++--------
> > tools/perf/builtin-trace.c | 5 +-
> > 3 files changed, 88 insertions(+), 111 deletions(-)
> >
> > --
> > 2.17.1
> >
--
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread