2015-04-24 17:01:16

by Davidlohr Bueso

[permalink] [raw]
Subject: [PATCH] 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(-)

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



2015-04-24 17:50:40

by David Ahern

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

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?

2015-04-24 17:57:46

by Davidlohr Bueso

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

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

Subject: [tip:perf/urgent] 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]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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);