* [PATCH 0/3] perf daemon/config: Fix a few minor issues
@ 2022-10-22 9:27 Yang Jihong
2022-10-22 9:27 ` [PATCH 1/3] perf daemon: Check err before calling setup_server_config in __cmd_start Yang Jihong
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Yang Jihong @ 2022-10-22 9:27 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, adrian.hunter, ak, linux-perf-users, linux-kernel
Cc: yangjihong1
Yang Jihong (3):
perf daemon: Check err before calling setup_server_config in
__cmd_start
perf daemon: Complete supported subcommand in help message
perf config: Add missing newline on pr_warning call in home_perfconfig
tools/perf/builtin-daemon.c | 4 ++--
tools/perf/util/config.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
--
2.30.GIT
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] perf daemon: Check err before calling setup_server_config in __cmd_start
2022-10-22 9:27 [PATCH 0/3] perf daemon/config: Fix a few minor issues Yang Jihong
@ 2022-10-22 9:27 ` Yang Jihong
2022-10-24 11:46 ` Arnaldo Carvalho de Melo
2022-10-22 9:27 ` [PATCH 2/3] perf daemon: Complete supported subcommand in help message Yang Jihong
2022-10-22 9:27 ` [PATCH 3/3] perf config: Add missing newline on pr_warning call in home_perfconfig Yang Jihong
2 siblings, 1 reply; 6+ messages in thread
From: Yang Jihong @ 2022-10-22 9:27 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, adrian.hunter, ak, linux-perf-users, linux-kernel
Cc: yangjihong1
If err!=0 before calling setup_server_config and reconfig==true,
setup_server_config function may return 0 and err becomes 0.
As a result, previous error is overwritten, need to check value of err first.
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
tools/perf/builtin-daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index 6cb3f6cc36d0..b82bd902602a 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -1333,7 +1333,7 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
if (fda.entries[signal_pos].revents & POLLIN)
err = handle_signalfd(daemon) < 0;
- if (reconfig)
+ if (!err && reconfig)
err = setup_server_config(daemon);
}
}
--
2.30.GIT
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] perf daemon: Complete supported subcommand in help message
2022-10-22 9:27 [PATCH 0/3] perf daemon/config: Fix a few minor issues Yang Jihong
2022-10-22 9:27 ` [PATCH 1/3] perf daemon: Check err before calling setup_server_config in __cmd_start Yang Jihong
@ 2022-10-22 9:27 ` Yang Jihong
2022-10-22 9:27 ` [PATCH 3/3] perf config: Add missing newline on pr_warning call in home_perfconfig Yang Jihong
2 siblings, 0 replies; 6+ messages in thread
From: Yang Jihong @ 2022-10-22 9:27 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, adrian.hunter, ak, linux-perf-users, linux-kernel
Cc: yangjihong1
perf daemon supports start, signale, stop and ping subcommands, complete it
Before:
# perf daemon -h
Usage: perf daemon start [<options>]
or: perf daemon [<options>]
-v, --verbose be more verbose
-x, --field-separator[=<field separator>]
print counts with custom separator
--base <directory>
base directory
--config <config file>
config file path
After:
# perf daemon -h
Usage: perf daemon {start|signal|stop|ping} [<options>]
or: perf daemon [<options>]
-v, --verbose be more verbose
-x, --field-separator[=<field separator>]
print counts with custom separator
--base <directory>
base directory
--config <config file>
config file path
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
tools/perf/builtin-daemon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index b82bd902602a..48698e7fd987 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -100,7 +100,7 @@ static struct daemon __daemon = {
};
static const char * const daemon_usage[] = {
- "perf daemon start [<options>]",
+ "perf daemon {start|signal|stop|ping} [<options>]",
"perf daemon [<options>]",
NULL
};
--
2.30.GIT
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] perf config: Add missing newline on pr_warning call in home_perfconfig
2022-10-22 9:27 [PATCH 0/3] perf daemon/config: Fix a few minor issues Yang Jihong
2022-10-22 9:27 ` [PATCH 1/3] perf daemon: Check err before calling setup_server_config in __cmd_start Yang Jihong
2022-10-22 9:27 ` [PATCH 2/3] perf daemon: Complete supported subcommand in help message Yang Jihong
@ 2022-10-22 9:27 ` Yang Jihong
2 siblings, 0 replies; 6+ messages in thread
From: Yang Jihong @ 2022-10-22 9:27 UTC (permalink / raw)
To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa,
namhyung, adrian.hunter, ak, linux-perf-users, linux-kernel
Cc: yangjihong1
fix missing newline on pr_warning call in home_perfconfig.
Before:
# perf record
File /home/yangjihong/.perfconfig not owned by current user or root, ignoring it.Couldn't synthesize bpf events.
After:
# perf record
File /home/yangjihong/.perfconfig not owned by current user or root, ignoring it.
Couldn't synthesize bpf events.
Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
---
tools/perf/util/config.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 3f2ae19a1dd4..658170b8dcef 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -556,7 +556,7 @@ static char *home_perfconfig(void)
config = strdup(mkpath("%s/.perfconfig", home));
if (config == NULL) {
- pr_warning("Not enough memory to process %s/.perfconfig, ignoring it.", home);
+ pr_warning("Not enough memory to process %s/.perfconfig, ignoring it.\n", home);
return NULL;
}
@@ -564,7 +564,7 @@ static char *home_perfconfig(void)
goto out_free;
if (st.st_uid && (st.st_uid != geteuid())) {
- pr_warning("File %s not owned by current user or root, ignoring it.", config);
+ pr_warning("File %s not owned by current user or root, ignoring it.\n", config);
goto out_free;
}
--
2.30.GIT
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] perf daemon: Check err before calling setup_server_config in __cmd_start
2022-10-22 9:27 ` [PATCH 1/3] perf daemon: Check err before calling setup_server_config in __cmd_start Yang Jihong
@ 2022-10-24 11:46 ` Arnaldo Carvalho de Melo
2022-10-24 11:50 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-10-24 11:46 UTC (permalink / raw)
To: Jiri Olsa, Yang Jihong
Cc: peterz, mingo, mark.rutland, alexander.shishkin, namhyung,
adrian.hunter, ak, linux-perf-users, linux-kernel
Em Sat, Oct 22, 2022 at 05:27:33PM +0800, Yang Jihong escreveu:
> If err!=0 before calling setup_server_config and reconfig==true,
> setup_server_config function may return 0 and err becomes 0.
> As a result, previous error is overwritten, need to check value of err first.
>
> Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> ---
> tools/perf/builtin-daemon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> index 6cb3f6cc36d0..b82bd902602a 100644
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -1333,7 +1333,7 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
> if (fda.entries[signal_pos].revents & POLLIN)
> err = handle_signalfd(daemon) < 0;
>
> - if (reconfig)
> + if (!err && reconfig)
> err = setup_server_config(daemon);
> }
Expanding the context a bit:
while (!done && !err) {
err = daemon__reconfig(daemon);
if (!err && fdarray__poll(&fda, -1)) {
bool reconfig = false;
if (fda.entries[sock_pos].revents & POLLIN)
err = handle_server_socket(daemon, sock_fd);
if (fda.entries[file_pos].revents & POLLIN)
err = handle_config_changes(daemon, conf_fd, &reconfig);
if (fda.entries[signal_pos].revents & POLLIN)
err = handle_signalfd(daemon) < 0;
if (!err && reconfig)
err = setup_server_config(daemon);
}
}
The err you're checking may be the last one, that may have overwritten
'err' from handle_config_changes(), perhaps moving things around helps?
I.e.:
if (!err && fdarray__poll(&fda, -1)) {
bool reconfig = false;
if (fda.entries[sock_pos].revents & POLLIN)
err = handle_server_socket(daemon, sock_fd);
if (fda.entries[signal_pos].revents & POLLIN)
err = handle_signalfd(daemon) < 0;
if (fda.entries[file_pos].revents & POLLIN)
err = handle_config_changes(daemon, conf_fd, &reconfig);
if (!err && reconfig)
err = setup_server_config(daemon);
}
?
Jiri?
- Arnaldo
> }
> --
> 2.30.GIT
--
- Arnaldo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] perf daemon: Check err before calling setup_server_config in __cmd_start
2022-10-24 11:46 ` Arnaldo Carvalho de Melo
@ 2022-10-24 11:50 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-10-24 11:50 UTC (permalink / raw)
To: Jiri Olsa, Yang Jihong
Cc: peterz, mingo, mark.rutland, alexander.shishkin, namhyung,
adrian.hunter, ak, linux-perf-users, linux-kernel
Em Mon, Oct 24, 2022 at 08:46:44AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Sat, Oct 22, 2022 at 05:27:33PM +0800, Yang Jihong escreveu:
> > If err!=0 before calling setup_server_config and reconfig==true,
> > setup_server_config function may return 0 and err becomes 0.
> > As a result, previous error is overwritten, need to check value of err first.
Applied patches 2 and 3 meanwhile, thanks.
- Arnaldo
> > Signed-off-by: Yang Jihong <yangjihong1@huawei.com>
> > ---
> > tools/perf/builtin-daemon.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> > index 6cb3f6cc36d0..b82bd902602a 100644
> > --- a/tools/perf/builtin-daemon.c
> > +++ b/tools/perf/builtin-daemon.c
> > @@ -1333,7 +1333,7 @@ static int __cmd_start(struct daemon *daemon, struct option parent_options[],
> > if (fda.entries[signal_pos].revents & POLLIN)
> > err = handle_signalfd(daemon) < 0;
> >
> > - if (reconfig)
> > + if (!err && reconfig)
> > err = setup_server_config(daemon);
> > }
>
> Expanding the context a bit:
>
> while (!done && !err) {
> err = daemon__reconfig(daemon);
>
> if (!err && fdarray__poll(&fda, -1)) {
> bool reconfig = false;
>
> if (fda.entries[sock_pos].revents & POLLIN)
> err = handle_server_socket(daemon, sock_fd);
> if (fda.entries[file_pos].revents & POLLIN)
> err = handle_config_changes(daemon, conf_fd, &reconfig);
> if (fda.entries[signal_pos].revents & POLLIN)
> err = handle_signalfd(daemon) < 0;
>
> if (!err && reconfig)
> err = setup_server_config(daemon);
> }
> }
>
> The err you're checking may be the last one, that may have overwritten
> 'err' from handle_config_changes(), perhaps moving things around helps?
> I.e.:
> if (!err && fdarray__poll(&fda, -1)) {
> bool reconfig = false;
>
> if (fda.entries[sock_pos].revents & POLLIN)
> err = handle_server_socket(daemon, sock_fd);
> if (fda.entries[signal_pos].revents & POLLIN)
> err = handle_signalfd(daemon) < 0;
> if (fda.entries[file_pos].revents & POLLIN)
> err = handle_config_changes(daemon, conf_fd, &reconfig);
>
> if (!err && reconfig)
> err = setup_server_config(daemon);
> }
>
>
> ?
>
> Jiri?
>
> - Arnaldo
>
> > }
> > --
> > 2.30.GIT
>
> --
>
> - Arnaldo
--
- Arnaldo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-24 16:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-22 9:27 [PATCH 0/3] perf daemon/config: Fix a few minor issues Yang Jihong
2022-10-22 9:27 ` [PATCH 1/3] perf daemon: Check err before calling setup_server_config in __cmd_start Yang Jihong
2022-10-24 11:46 ` Arnaldo Carvalho de Melo
2022-10-24 11:50 ` Arnaldo Carvalho de Melo
2022-10-22 9:27 ` [PATCH 2/3] perf daemon: Complete supported subcommand in help message Yang Jihong
2022-10-22 9:27 ` [PATCH 3/3] perf config: Add missing newline on pr_warning call in home_perfconfig Yang Jihong
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).