* [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
* 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
* [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
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).