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

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

[Davidlohr Bueso]

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 <dbueso@suse.de>
---
 tools/perf/bench/futex-requeue.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/perf/bench/futex-requeue.c b/tools/perf/bench/futex-requeue.c
index bedff6b..ad0d9b5 100644
--- a/tools/perf/bench/futex-requeue.c
+++ b/tools/perf/bench/futex-requeue.c
@@ -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);
 
-- 
2.1.4

Re: [PATCH] perf-bench/futex: Fix hung wakeup tasks after requeueing

[David Ahern]

On 4/24/15 11:00 AM, Davidlohr Bueso wrote:
> 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?

Re: [PATCH] perf-bench/futex: Fix hung wakeup tasks after requeueing

[Davidlohr Bueso]

On Fri, 2015-04-24 at 11:50 -0600, David Ahern wrote:
> On 4/24/15 11:00 AM, Davidlohr Bueso wrote:
> > 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

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

[tip-bot for Davidlohr Bueso]

Commit-ID:  052b0f6eaf8b1f02669884a177bc3ce463133a42
Gitweb:     http://git.kernel.org/tip/052b0f6eaf8b1f02669884a177bc3ce463133a42
Author:     Davidlohr Bueso <dave@stgolabs.net>
AuthorDate: Fri, 24 Apr 2015 10:00:48 -0700
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
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 <dbueso@suse.de>
Cc: Mel Gorman <mgorman@suse.de>
Link: http://lkml.kernel.org/r/1429894848.10273.44.camel@stgolabs.net
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/bench/futex-requeue.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tools/perf/bench/futex-requeue.c b/tools/perf/bench/futex-requeue.c
index bedff6b..ad0d9b5 100644
--- a/tools/perf/bench/futex-requeue.c
+++ b/tools/perf/bench/futex-requeue.c
@@ -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);