Re: Possible overwriting of errno

[Jiri Olsa <olsajiri@gmail.com>]

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;
 		}