Skip to contentSkip to conversation

perf-bench/futex: Fix hung wakeup tasks after requeueing

PATCH In review Davidlohr Bueso 3 participants 4 messages Apr 24, 2015 – May 1, 2015

Subsystems (2)
perf events Futex
lore
Share & tools
lore

Davidlohr Bueso

author reviewer
#0 lore raw

PATCH 1/1 perf-bench/futex: Fix hung wakeup tasks after requeueing

The futex-requeue benchmark can hang because of missing wakeups once the benchmark is done, ie:

[Run 1]: Requeued 1024 of 1024 threads in 0.3290 ms
perf: couldn't wakeup all tasks (135/1024)

This bug, while perhaps suggesting missing wakeups in kernel futex code, is merely a consequence of the crappy FUTEX_CMP_REQUEUE man page, incorrectly mentioning that the number of requeued tasks is in fact returned, not the wakeups. This patch acknowledges this and updates the corresponding futex_wake code around it.

Signed-off-by Davidlohr Bueso <[email protected]>
---
 tools/perf/bench/futex-requeue.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
tools/perf/bench/futex-requeue.c +8 -7
index bedff6b..ad0d9b5 100644
@@ -132,6 +132,9 @@ int bench_futex_requeue(int argc, const char **argv,
 	if (!fshared)
 		futex_flag = FUTEX_PRIVATE_FLAG;
 
+	if (nrequeue > nthreads)
+		nrequeue = nthreads;
+
 	printf("Run summary [PID %d]: Requeuing %d threads (from [%s] %p to %p), "
 	       "%d at a time.\n\n",  getpid(), nthreads,
 	       fshared ? "shared":"private", &futex1, &futex2, nrequeue);

@@ -161,20 +164,18 @@ int bench_futex_requeue(int argc, const char **argv,
 
 		/* Ok, all threads are patiently blocked, start requeueing */
 		gettimeofday(&start, NULL);
-		for (nrequeued = 0; nrequeued < nthreads; nrequeued += nrequeue) {
+		while (nrequeued < nthreads) {
 			/*
 			 * Do not wakeup any tasks blocked on futex1, allowing
 			 * us to really measure futex_wait functionality.
 			 */
-			futex_cmp_requeue(&futex1, 0, &futex2, 0,
-					  nrequeue, futex_flag);
+			nrequeued += futex_cmp_requeue(&futex1, 0, &futex2, 0,
+						       nrequeue, futex_flag);
 		}
+
 		gettimeofday(&end, NULL);
 		timersub(&end, &start, &runtime);
 
-		if (nrequeued > nthreads)
-			nrequeued = nthreads;
-
 		update_stats(&requeued_stats, nrequeued);
 		update_stats(&requeuetime_stats, runtime.tv_usec);
 

@@ -184,7 +185,7 @@ int bench_futex_requeue(int argc, const char **argv,
 		}
 
 		/* everybody should be blocked on futex2, wake'em up */
-		nrequeued = futex_wake(&futex2, nthreads, futex_flag);
+		nrequeued = futex_wake(&futex2, nrequeued, futex_flag);
 		if (nthreads != nrequeued)
 			warnx("couldn't wakeup all tasks (%d/%d)", nrequeued, nthreads);
Signature · 1 line
2.1.4
#1 lore raw
Quoted 5 lines · Davidlohr Bueso

This bug, while perhaps suggesting missing wakeups in kernel futex code, is merely a consequence of the crappy FUTEX_CMP_REQUEUE man page, incorrectly mentioning that the number of requeued tasks is in fact returned, not the wakeups. This patch acknowledges this and updates the corresponding futex_wake code around it.

Did you submit an update to fix the crappy man page?

author reviewer
#2 lore raw
Quoted 8 lines · David Ahern
Quoted 5 lines · Davidlohr Bueso

This bug, while perhaps suggesting missing wakeups in kernel futex code, is merely a consequence of the crappy FUTEX_CMP_REQUEUE man page, incorrectly mentioning that the number of requeued tasks is in fact returned, not the wakeups. This patch acknowledges this and updates the corresponding futex_wake code around it.

Did you submit an update to fix the crappy man page?

There is a major manpage rewrite going on. Hopefully Michael will merge it soon.

Thanks,
Davidlohr

merge bot
#3 lore raw

[tip:perf/urgent] perf bench futex: Fix hung wakeup tasks after requeueing

In reply to Davidlohr Bueso
perf bench futex: Fix hung wakeup tasks after requeueing
Commit-ID:  052b0f6eaf8b1f02669884a177bc3ce463133a42
Gitweb:     http://git.kernel.org/tip/052b0f6eaf8b1f02669884a177bc3ce463133a42
Author:     Davidlohr Bueso <[email protected]>
AuthorDate: Fri, 24 Apr 2015 10:00:48 -0700
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 27 Apr 2015 13:57:49 -0300

perf bench futex: Fix hung wakeup tasks after requeueing

The futex-requeue benchmark can hang because of missing wakeups once the benchmark is done, ie:

[Run 1]: Requeued 1024 of 1024 threads in 0.3290 ms
perf: couldn't wakeup all tasks (135/1024)

This bug, while perhaps suggesting missing wakeups in kernel futex code, is merely a consequence of the crappy FUTEX_CMP_REQUEUE man page, incorrectly mentioning that the number of requeued tasks is in fact returned, not the wakeups.

This patch acknowledges this and updates the corresponding futex_wake code around it.

Signed-off-by Davidlohr Bueso <[email protected]>
Cc Mel Gorman <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 tools/perf/bench/futex-requeue.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)
tools/perf/bench/futex-requeue.c +8 -7
index bedff6b..ad0d9b5 100644
@@ -132,6 +132,9 @@ int bench_futex_requeue(int argc, const char **argv,
 	if (!fshared)
 		futex_flag = FUTEX_PRIVATE_FLAG;
 
+	if (nrequeue > nthreads)
+		nrequeue = nthreads;
+
 	printf("Run summary [PID %d]: Requeuing %d threads (from [%s] %p to %p), "
 	       "%d at a time.\n\n",  getpid(), nthreads,
 	       fshared ? "shared":"private", &futex1, &futex2, nrequeue);

@@ -161,20 +164,18 @@ int bench_futex_requeue(int argc, const char **argv,
 
 		/* Ok, all threads are patiently blocked, start requeueing */
 		gettimeofday(&start, NULL);
-		for (nrequeued = 0; nrequeued < nthreads; nrequeued += nrequeue) {
+		while (nrequeued < nthreads) {
 			/*
 			 * Do not wakeup any tasks blocked on futex1, allowing
 			 * us to really measure futex_wait functionality.
 			 */
-			futex_cmp_requeue(&futex1, 0, &futex2, 0,
-					  nrequeue, futex_flag);
+			nrequeued += futex_cmp_requeue(&futex1, 0, &futex2, 0,
+						       nrequeue, futex_flag);
 		}
+
 		gettimeofday(&end, NULL);
 		timersub(&end, &start, &runtime);
 
-		if (nrequeued > nthreads)
-			nrequeued = nthreads;
-
 		update_stats(&requeued_stats, nrequeued);
 		update_stats(&requeuetime_stats, runtime.tv_usec);
 

@@ -184,7 +185,7 @@ int bench_futex_requeue(int argc, const char **argv,
 		}
 
 		/* everybody should be blocked on futex2, wake'em up */
-		nrequeued = futex_wake(&futex2, nthreads, futex_flag);
+		nrequeued = futex_wake(&futex2, nrequeued, futex_flag);
 		if (nthreads != nrequeued)
 			warnx("couldn't wakeup all tasks (%d/%d)", nrequeued, nthreads);