* [PATCH 0/4] perf: Some clean up
@ 2022-09-22 14:14 Shang XiaoJing
2022-09-22 14:14 ` [PATCH 1/4] perf genelf: Fix error code in jit_write_elf() Shang XiaoJing
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Shang XiaoJing @ 2022-09-22 14:14 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, linux-perf-users
Cc: shangxiaojing
Some clean ups in tools/perf.
Shang XiaoJing (4):
perf genelf: Fix error code in jit_write_elf()
perf stat: Merge cases in process_evlist
perf top: Fix error code in cmd_top()
perf stat: Clean redundant if in process_evlist
tools/perf/builtin-stat.c | 5 -----
tools/perf/builtin-top.c | 3 +++
tools/perf/util/genelf.c | 1 +
3 files changed, 4 insertions(+), 5 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] perf genelf: Fix error code in jit_write_elf()
2022-09-22 14:14 [PATCH 0/4] perf: Some clean up Shang XiaoJing
@ 2022-09-22 14:14 ` Shang XiaoJing
2022-09-22 19:27 ` Arnaldo Carvalho de Melo
2022-09-22 14:14 ` [PATCH 2/4] perf stat: Merge cases in process_evlist Shang XiaoJing
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Shang XiaoJing @ 2022-09-22 14:14 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, linux-perf-users
Cc: shangxiaojing
The error code is set to -1 at the beginning of jit_write_elf(), but it is
assigned by jit_add_eh_frame_info() in the middle, hence the following
error can only return the error code of jit_add_eh_frame_info(). Reset
the error code to the default value after being assigned by
jit_add_eh_frame_info().
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
tools/perf/util/genelf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
index ed28a0dbcb7f..2e45b204494a 100644
--- a/tools/perf/util/genelf.c
+++ b/tools/perf/util/genelf.c
@@ -331,6 +331,7 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
eh_frame_base_offset);
if (retval)
goto error;
+ retval = -1;
}
/*
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/4] perf stat: Merge cases in process_evlist
2022-09-22 14:14 [PATCH 0/4] perf: Some clean up Shang XiaoJing
2022-09-22 14:14 ` [PATCH 1/4] perf genelf: Fix error code in jit_write_elf() Shang XiaoJing
@ 2022-09-22 14:14 ` Shang XiaoJing
2022-09-22 19:33 ` Arnaldo Carvalho de Melo
2022-09-22 14:14 ` [PATCH 3/4] perf top: Fix error code in cmd_top() Shang XiaoJing
2022-09-22 14:14 ` [PATCH 4/4] perf stat: Clean redundant if in process_evlist Shang XiaoJing
3 siblings, 1 reply; 10+ messages in thread
From: Shang XiaoJing @ 2022-09-22 14:14 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, linux-perf-users
Cc: shangxiaojing
As two cases in process_evlist has same behavior, merge these cases.
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
tools/perf/builtin-stat.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index e05fe72c1d87..e10595f649bc 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -662,9 +662,6 @@ static void process_evlist(struct evlist *evlist, unsigned int interval)
if (evlist__ctlfd_process(evlist, &cmd) > 0) {
switch (cmd) {
case EVLIST_CTL_CMD_ENABLE:
- if (interval)
- process_interval();
- break;
case EVLIST_CTL_CMD_DISABLE:
if (interval)
process_interval();
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/4] perf top: Fix error code in cmd_top()
2022-09-22 14:14 [PATCH 0/4] perf: Some clean up Shang XiaoJing
2022-09-22 14:14 ` [PATCH 1/4] perf genelf: Fix error code in jit_write_elf() Shang XiaoJing
2022-09-22 14:14 ` [PATCH 2/4] perf stat: Merge cases in process_evlist Shang XiaoJing
@ 2022-09-22 14:14 ` Shang XiaoJing
2022-09-22 14:14 ` [PATCH 4/4] perf stat: Clean redundant if in process_evlist Shang XiaoJing
3 siblings, 0 replies; 10+ messages in thread
From: Shang XiaoJing @ 2022-09-22 14:14 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, linux-perf-users
Cc: shangxiaojing
There are three error paths which return success:
1. Propagate the errno from evlist__create_maps() if it failed.
2. Return -EINVAL if top.sb_evlist is NULL.
3. Return -EINVAL if evlist__add_bpf_sb_event() failed.
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
tools/perf/builtin-top.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index e89208b4ad4b..4b3ff7687236 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1707,6 +1707,7 @@ int cmd_top(int argc, const char **argv)
if (evlist__create_maps(top.evlist, target) < 0) {
ui__error("Couldn't create thread/CPU maps: %s\n",
errno == ENOENT ? "No such process" : str_error_r(errno, errbuf, sizeof(errbuf)));
+ status = -errno;
goto out_delete_evlist;
}
@@ -1759,11 +1760,13 @@ int cmd_top(int argc, const char **argv)
if (top.sb_evlist == NULL) {
pr_err("Couldn't create side band evlist.\n.");
+ status = -EINVAL;
goto out_delete_evlist;
}
if (evlist__add_bpf_sb_event(top.sb_evlist, &perf_env)) {
pr_err("Couldn't ask for PERF_RECORD_BPF_EVENT side band events.\n.");
+ status = -EINVAL;
goto out_delete_evlist;
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/4] perf stat: Clean redundant if in process_evlist
2022-09-22 14:14 [PATCH 0/4] perf: Some clean up Shang XiaoJing
` (2 preceding siblings ...)
2022-09-22 14:14 ` [PATCH 3/4] perf top: Fix error code in cmd_top() Shang XiaoJing
@ 2022-09-22 14:14 ` Shang XiaoJing
2022-09-22 19:54 ` Arnaldo Carvalho de Melo
3 siblings, 1 reply; 10+ messages in thread
From: Shang XiaoJing @ 2022-09-22 14:14 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, linux-perf-users
Cc: shangxiaojing
Since the first if statment is covered by the following one, clean up
the first if statment.
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
tools/perf/builtin-stat.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index e10595f649bc..ccea5d1e053a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -899,8 +899,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
counter = evlist_cpu_itr.evsel;
- if (!counter->reset_group && !counter->errored)
- continue;
if (!counter->reset_group)
continue;
try_again_reset:
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] perf genelf: Fix error code in jit_write_elf()
2022-09-22 14:14 ` [PATCH 1/4] perf genelf: Fix error code in jit_write_elf() Shang XiaoJing
@ 2022-09-22 19:27 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-09-22 19:27 UTC (permalink / raw)
To: Shang XiaoJing
Cc: Stefano Sanfilippo, peterz, mingo, mark.rutland,
alexander.shishkin, jolsa, namhyung, linux-perf-users
Em Thu, Sep 22, 2022 at 10:14:35PM +0800, Shang XiaoJing escreveu:
> The error code is set to -1 at the beginning of jit_write_elf(), but it is
> assigned by jit_add_eh_frame_info() in the middle, hence the following
> error can only return the error code of jit_add_eh_frame_info(). Reset
> the error code to the default value after being assigned by
> jit_add_eh_frame_info().
You forgot to add:
Cc: Stefano Sanfilippo <ssanfilippo@chromium.org>
Fixes: 086f9f3d7897d808 ("perf jit: Generate .eh_frame/.eh_frame_hdr in DSO")
Applied.
- Arnaldo
> Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
> ---
> tools/perf/util/genelf.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
> index ed28a0dbcb7f..2e45b204494a 100644
> --- a/tools/perf/util/genelf.c
> +++ b/tools/perf/util/genelf.c
> @@ -331,6 +331,7 @@ jit_write_elf(int fd, uint64_t load_addr, const char *sym,
> eh_frame_base_offset);
> if (retval)
> goto error;
> + retval = -1;
> }
>
> /*
> --
> 2.17.1
--
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] perf stat: Merge cases in process_evlist
2022-09-22 14:14 ` [PATCH 2/4] perf stat: Merge cases in process_evlist Shang XiaoJing
@ 2022-09-22 19:33 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-09-22 19:33 UTC (permalink / raw)
To: Shang XiaoJing
Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
linux-perf-users
Em Thu, Sep 22, 2022 at 10:14:36PM +0800, Shang XiaoJing escreveu:
> As two cases in process_evlist has same behavior, merge these cases.
Applied.
Added the patch below, see:
include/linux/compiler_attributes.h and
tools/include/linux/compiler-gcc.h.
- Arnaldo
Commiter notes:
Added __fallthrough, the kernel has "fallthrough", we need to make
tools/ use it.
---
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index e10595f649bcb2b4..7b8e901bce101b63 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -662,6 +662,7 @@ static void process_evlist(struct evlist *evlist, unsigned int interval)
if (evlist__ctlfd_process(evlist, &cmd) > 0) {
switch (cmd) {
case EVLIST_CTL_CMD_ENABLE:
+ __fallthrough;
case EVLIST_CTL_CMD_DISABLE:
if (interval)
process_interval();
> Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
> ---
> tools/perf/builtin-stat.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index e05fe72c1d87..e10595f649bc 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -662,9 +662,6 @@ static void process_evlist(struct evlist *evlist, unsigned int interval)
> if (evlist__ctlfd_process(evlist, &cmd) > 0) {
> switch (cmd) {
> case EVLIST_CTL_CMD_ENABLE:
> - if (interval)
> - process_interval();
> - break;
> case EVLIST_CTL_CMD_DISABLE:
> if (interval)
> process_interval();
> --
> 2.17.1
--
- Arnaldo
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] perf stat: Clean redundant if in process_evlist
2022-09-22 14:14 ` [PATCH 4/4] perf stat: Clean redundant if in process_evlist Shang XiaoJing
@ 2022-09-22 19:54 ` Arnaldo Carvalho de Melo
2022-09-22 22:50 ` Ian Rogers
0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-09-22 19:54 UTC (permalink / raw)
To: Ian Rogers, Shang XiaoJing
Cc: peterz, mingo, mark.rutland, alexander.shishkin, jolsa, namhyung,
linux-perf-users
Em Thu, Sep 22, 2022 at 10:14:38PM +0800, Shang XiaoJing escreveu:
> Since the first if statment is covered by the following one, clean up
> the first if statment.
>
> Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
> ---
> tools/perf/builtin-stat.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index e10595f649bc..ccea5d1e053a 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -899,8 +899,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
> counter = evlist_cpu_itr.evsel;
>
> - if (!counter->reset_group && !counter->errored)
> - continue;
> if (!counter->reset_group)
> continue;
Ian, can you please take a look at this one? You added this in:
commit 472832d2c000b9611feaea66fe521055c3dbf17a
Author: Ian Rogers <irogers@google.com>
Date: Tue Jan 4 22:13:37 2022 -0800
perf evlist: Refactor evlist__for_each_cpu()
> try_again_reset:
> --
> 2.17.1
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] perf stat: Clean redundant if in process_evlist
2022-09-22 19:54 ` Arnaldo Carvalho de Melo
@ 2022-09-22 22:50 ` Ian Rogers
2022-09-26 20:32 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 10+ messages in thread
From: Ian Rogers @ 2022-09-22 22:50 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Shang XiaoJing, peterz, mingo, mark.rutland, alexander.shishkin,
jolsa, namhyung, linux-perf-users
On Thu, Sep 22, 2022 at 12:55 PM Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
>
> Em Thu, Sep 22, 2022 at 10:14:38PM +0800, Shang XiaoJing escreveu:
> > Since the first if statment is covered by the following one, clean up
> > the first if statment.
> >
> > Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
Acked-by: Ian Rogers <irogers@google.com>
Thanks,
Ian
> > ---
> > tools/perf/builtin-stat.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index e10595f649bc..ccea5d1e053a 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -899,8 +899,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> > evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
> > counter = evlist_cpu_itr.evsel;
> >
> > - if (!counter->reset_group && !counter->errored)
> > - continue;
> > if (!counter->reset_group)
> > continue;
>
> Ian, can you please take a look at this one? You added this in:
>
> commit 472832d2c000b9611feaea66fe521055c3dbf17a
> Author: Ian Rogers <irogers@google.com>
> Date: Tue Jan 4 22:13:37 2022 -0800
>
> perf evlist: Refactor evlist__for_each_cpu()
>
> > try_again_reset:
> > --
> > 2.17.1
>
>
> - Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] perf stat: Clean redundant if in process_evlist
2022-09-22 22:50 ` Ian Rogers
@ 2022-09-26 20:32 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-09-26 20:32 UTC (permalink / raw)
To: Ian Rogers
Cc: Shang XiaoJing, peterz, mingo, mark.rutland, alexander.shishkin,
jolsa, namhyung, linux-perf-users
Em Thu, Sep 22, 2022 at 03:50:40PM -0700, Ian Rogers escreveu:
> On Thu, Sep 22, 2022 at 12:55 PM Arnaldo Carvalho de Melo
> <acme@kernel.org> wrote:
> >
> > Em Thu, Sep 22, 2022 at 10:14:38PM +0800, Shang XiaoJing escreveu:
> > > Since the first if statment is covered by the following one, clean up
> > > the first if statment.
> > >
> > > Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
>
> Acked-by: Ian Rogers <irogers@google.com>
Thanks, applied.
- Arnaldo
> Thanks,
> Ian
>
> > > ---
> > > tools/perf/builtin-stat.c | 2 --
> > > 1 file changed, 2 deletions(-)
> > >
> > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > > index e10595f649bc..ccea5d1e053a 100644
> > > --- a/tools/perf/builtin-stat.c
> > > +++ b/tools/perf/builtin-stat.c
> > > @@ -899,8 +899,6 @@ static int __run_perf_stat(int argc, const char **argv, int run_idx)
> > > evlist__for_each_cpu(evlist_cpu_itr, evsel_list, affinity) {
> > > counter = evlist_cpu_itr.evsel;
> > >
> > > - if (!counter->reset_group && !counter->errored)
> > > - continue;
> > > if (!counter->reset_group)
> > > continue;
> >
> > Ian, can you please take a look at this one? You added this in:
> >
> > commit 472832d2c000b9611feaea66fe521055c3dbf17a
> > Author: Ian Rogers <irogers@google.com>
> > Date: Tue Jan 4 22:13:37 2022 -0800
> >
> > perf evlist: Refactor evlist__for_each_cpu()
> >
> > > try_again_reset:
> > > --
> > > 2.17.1
> >
> >
> > - Arnaldo
--
- Arnaldo
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-09-26 20:33 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 14:14 [PATCH 0/4] perf: Some clean up Shang XiaoJing
2022-09-22 14:14 ` [PATCH 1/4] perf genelf: Fix error code in jit_write_elf() Shang XiaoJing
2022-09-22 19:27 ` Arnaldo Carvalho de Melo
2022-09-22 14:14 ` [PATCH 2/4] perf stat: Merge cases in process_evlist Shang XiaoJing
2022-09-22 19:33 ` Arnaldo Carvalho de Melo
2022-09-22 14:14 ` [PATCH 3/4] perf top: Fix error code in cmd_top() Shang XiaoJing
2022-09-22 14:14 ` [PATCH 4/4] perf stat: Clean redundant if in process_evlist Shang XiaoJing
2022-09-22 19:54 ` Arnaldo Carvalho de Melo
2022-09-22 22:50 ` Ian Rogers
2022-09-26 20:32 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).