* Possible overwriting of errno
@ 2022-06-29 5:31 Emil Berg
2022-06-29 9:44 ` Jiri Olsa
2022-10-11 8:04 ` Emil Berg
0 siblings, 2 replies; 4+ messages in thread
From: Emil Berg @ 2022-06-29 5:31 UTC (permalink / raw)
To: linux-perf-users
Hi!
I'm getting the error "Failed to start threads: File exists" from perf, probably EEXIST.
I just want to discuss the changes to tools/perf/builtin-record.c made Feb 10 2022 with:
perf record: Start threads in the beginning of trace streaming
SHA: 3217e9fecf118d5dcabdd68d91e0c6afcb4c3e1b
At line 2014 pthread_create() is run and on line 2017 strerror(errno) is printed. Between line 2014 and 2017 record__terminate_thread() is run.
I just think record__terminate_thread() run in-between looks like it may overwrite errno, thus messing up the error message. To be clear I think the error message should come from failure of thread creation and not from failure of thread termination. Can someone enlighten me here?
if (pthread_create(&handle, &attrs, record__thread, &thread_data[t])) {
for (tt = 1; tt < t; tt++)
record__terminate_thread(&thread_data[t]);
pr_err("Failed to start threads: %s\n", strerror(errno));
/Emil Berg
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Possible overwriting of errno 2022-06-29 5:31 Possible overwriting of errno Emil Berg @ 2022-06-29 9:44 ` Jiri Olsa 2022-06-29 12:50 ` Emil Berg 2022-10-11 8:04 ` Emil Berg 1 sibling, 1 reply; 4+ messages in thread From: Jiri Olsa @ 2022-06-29 9:44 UTC (permalink / raw) To: Emil Berg; +Cc: linux-perf-users On Wed, Jun 29, 2022 at 05:31:42AM +0000, Emil Berg wrote: > Hi! > > I'm getting the error "Failed to start threads: File exists" from perf, probably EEXIST. > > I just want to discuss the changes to tools/perf/builtin-record.c made Feb 10 2022 with: > perf record: Start threads in the beginning of trace streaming > SHA: 3217e9fecf118d5dcabdd68d91e0c6afcb4c3e1b > > At line 2014 pthread_create() is run and on line 2017 strerror(errno) is printed. Between line 2014 and 2017 record__terminate_thread() is run. > > I just think record__terminate_thread() run in-between looks like it may overwrite errno, thus messing up the error message. To be clear I think the error message should come from failure of thread creation and not from failure of thread termination. Can someone enlighten me here? > > if (pthread_create(&handle, &attrs, record__thread, &thread_data[t])) { > for (tt = 1; tt < t; tt++) > record__terminate_thread(&thread_data[t]); > pr_err("Failed to start threads: %s\n", strerror(errno)); > > /Emil Berg yea, seems wrong.. could you try patch below? thanks, jirka --- diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index cf5c5379ceaa..158bb0f293d2 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -2103,11 +2103,11 @@ static int record__start_threads(struct record *rec) MMAP_CPU_MASK_BYTES(&(thread_data[t].mask->affinity)), (cpu_set_t *)(thread_data[t].mask->affinity.bits)); #endif - if (pthread_create(&handle, &attrs, record__thread, &thread_data[t])) { + ret = pthread_create(&handle, &attrs, record__thread, &thread_data[t]); + if (ret) { for (tt = 1; tt < t; tt++) record__terminate_thread(&thread_data[t]); - pr_err("Failed to start threads: %s\n", strerror(errno)); - ret = -1; + pr_err("Failed to start threads: %s\n", strerror(ret)); goto out_err; } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: Possible overwriting of errno 2022-06-29 9:44 ` Jiri Olsa @ 2022-06-29 12:50 ` Emil Berg 0 siblings, 0 replies; 4+ messages in thread From: Emil Berg @ 2022-06-29 12:50 UTC (permalink / raw) To: Jiri Olsa; +Cc: linux-perf-users That's nice. Right now it's a bit cumbersome to test this patch since I'm running perf via vtune in a container... > -----Original Message----- > From: Jiri Olsa <olsajiri@gmail.com> > Sent: den 29 juni 2022 11:45 > To: Emil Berg <emil.berg@ericsson.com> > Cc: linux-perf-users@vger.kernel.org > Subject: Re: Possible overwriting of errno > > On Wed, Jun 29, 2022 at 05:31:42AM +0000, Emil Berg wrote: > > Hi! > > > > I'm getting the error "Failed to start threads: File exists" from perf, > probably EEXIST. > > > > I just want to discuss the changes to tools/perf/builtin-record.c made Feb > 10 2022 with: > > perf record: Start threads in the beginning of trace streaming > > SHA: 3217e9fecf118d5dcabdd68d91e0c6afcb4c3e1b > > > > At line 2014 pthread_create() is run and on line 2017 strerror(errno) is > printed. Between line 2014 and 2017 record__terminate_thread() is run. > > > > I just think record__terminate_thread() run in-between looks like it may > overwrite errno, thus messing up the error message. To be clear I think the > error message should come from failure of thread creation and not from > failure of thread termination. Can someone enlighten me here? > > > > if (pthread_create(&handle, &attrs, record__thread, &thread_data[t])) { > > for (tt = 1; tt < t; tt++) > > record__terminate_thread(&thread_data[t]); > > pr_err("Failed to start threads: %s\n", strerror(errno)); > > > > /Emil Berg > > yea, seems wrong.. could you try patch below? > > thanks, > jirka > > > --- > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index > cf5c5379ceaa..158bb0f293d2 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -2103,11 +2103,11 @@ static int record__start_threads(struct record > *rec) > > MMAP_CPU_MASK_BYTES(&(thread_data[t].mask->affinity)), > > (cpu_set_t *)(thread_data[t].mask->affinity.bits)); > #endif > - if (pthread_create(&handle, &attrs, > record__thread, &thread_data[t])) { > + ret = pthread_create(&handle, &attrs, > record__thread, &thread_data[t]); > + if (ret) { > for (tt = 1; tt < t; tt++) > > record__terminate_thread(&thread_data[t]); > - pr_err("Failed to start threads: > %s\n", strerror(errno)); > - ret = -1; > + pr_err("Failed to start threads: > %s\n", strerror(ret)); > goto out_err; > } > ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: Possible overwriting of errno 2022-06-29 5:31 Possible overwriting of errno Emil Berg 2022-06-29 9:44 ` Jiri Olsa @ 2022-10-11 8:04 ` Emil Berg 1 sibling, 0 replies; 4+ messages in thread From: Emil Berg @ 2022-10-11 8:04 UTC (permalink / raw) To: linux-perf-users Although it's been a while I still think this is a bug. Assuming any posix function, e.g. close() in record__terminate_thread(), can modify errno, it seems like this is an issue which may produce confusing error messages. Will there be a fix for this? See below. > -----Original Message----- > From: Emil Berg > Sent: den 29 juni 2022 07:32 > To: linux-perf-users@vger.kernel.org > Subject: Possible overwriting of errno > > Hi! > > I'm getting the error "Failed to start threads: File exists" from perf, probably > EEXIST. > > I just want to discuss the changes to tools/perf/builtin-record.c made Feb 10 > 2022 with: > perf record: Start threads in the beginning of trace streaming > SHA: 3217e9fecf118d5dcabdd68d91e0c6afcb4c3e1b > > At line 2014 pthread_create() is run and on line 2017 strerror(errno) is > printed. Between line 2014 and 2017 record__terminate_thread() is run. > > I just think record__terminate_thread() run in-between looks like it may > overwrite errno, thus messing up the error message. To be clear I think the > error message should come from failure of thread creation and not from > failure of thread termination. Can someone enlighten me here? > > if (pthread_create(&handle, &attrs, record__thread, &thread_data[t])) { > for (tt = 1; tt < t; tt++) > record__terminate_thread(&thread_data[t]); > pr_err("Failed to start threads: %s\n", strerror(errno)); > > /Emil Berg ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-11 8:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-29 5:31 Possible overwriting of errno Emil Berg 2022-06-29 9:44 ` Jiri Olsa 2022-06-29 12:50 ` Emil Berg 2022-10-11 8:04 ` Emil Berg
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.