[PATCH] sched: Wake up all non-exclusive waiters in __wake_up_common()

[PATCH] sched: Wake up all non-exclusive waiters in __wake_up_common()

[Byungchul Park]

__wake_up_common() should wake up all non-exclusive waiters and
exclusive waiters as many as nr_exclusive, but currently it does not.

Consider a wait queue like the following for example:

   A(exclusive) -> B(non-exclusive) -> C(non-exclusive)

Current code will wake up only A when nr_exclusive = 1, but has to wake
up A, B and C. Make it do as we expect.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 kernel/sched/wait.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 9453efe..0ea1083 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -67,12 +67,23 @@ static void __wake_up_common(wait_queue_head_t *q, unsigned int mode,
 {
 	wait_queue_t *curr, *next;
 
+	/*
+	 * We use nr_exclusive = 0 to wake up all no matter whether
+	 * WQ_FLAG_EXCLUSIVE is set. However, we have to distinguish
+	 * between the case and having finished all exclusive wake-up.
+	 * So make nr_exclusive non-zero in advance in the former case.
+	 */
+	nr_exclusive = nr_exclusive ?: -1;
+
 	list_for_each_entry_safe(curr, next, &q->task_list, task_list) {
 		unsigned flags = curr->flags;
 
+		if ((flags & WQ_FLAG_EXCLUSIVE) && !nr_exclusive)
+			continue;
+
 		if (curr->func(curr, mode, wake_flags, key) &&
-				(flags & WQ_FLAG_EXCLUSIVE) && !--nr_exclusive)
-			break;
+		    (flags & WQ_FLAG_EXCLUSIVE))
+			nr_exclusive--;
 	}
 }
 
-- 
1.9.1

Re: [PATCH] sched: Wake up all non-exclusive waiters in __wake_up_common()

[Byungchul Park]

On Wed, Mar 08, 2017 at 09:21:52AM +0900, Byungchul Park wrote:
> __wake_up_common() should wake up all non-exclusive waiters and
> exclusive waiters as many as nr_exclusive, but currently it does not.
> 
> Consider a wait queue like the following for example:
> 
>    A(exclusive) -> B(non-exclusive) -> C(non-exclusive)
> 
> Current code will wake up only A when nr_exclusive = 1, but has to wake
> up A, B and C. Make it do as we expect.

I'm wondering if I was wrong. Am I wrong?

> 
> Signed-off-by: Byungchul Park <byungchul.park@lge.com>
> ---
>  kernel/sched/wait.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
> index 9453efe..0ea1083 100644
> --- a/kernel/sched/wait.c
> +++ b/kernel/sched/wait.c
> @@ -67,12 +67,23 @@ static void __wake_up_common(wait_queue_head_t *q, unsigned int mode,
>  {
>  	wait_queue_t *curr, *next;
>  
> +	/*
> +	 * We use nr_exclusive = 0 to wake up all no matter whether
> +	 * WQ_FLAG_EXCLUSIVE is set. However, we have to distinguish
> +	 * between the case and having finished all exclusive wake-up.
> +	 * So make nr_exclusive non-zero in advance in the former case.
> +	 */
> +	nr_exclusive = nr_exclusive ?: -1;
> +
>  	list_for_each_entry_safe(curr, next, &q->task_list, task_list) {
>  		unsigned flags = curr->flags;
>  
> +		if ((flags & WQ_FLAG_EXCLUSIVE) && !nr_exclusive)
> +			continue;
> +
>  		if (curr->func(curr, mode, wake_flags, key) &&
> -				(flags & WQ_FLAG_EXCLUSIVE) && !--nr_exclusive)
> -			break;
> +		    (flags & WQ_FLAG_EXCLUSIVE))
> +			nr_exclusive--;
>  	}
>  }
>  
> -- 
> 1.9.1

Re: [PATCH] sched: Wake up all non-exclusive waiters in __wake_up_common()

[Peter Zijlstra]

On Wed, Mar 08, 2017 at 09:21:52AM +0900, Byungchul Park wrote:
> __wake_up_common() should wake up all non-exclusive waiters and
> exclusive waiters as many as nr_exclusive, but currently it does not.
> 
> Consider a wait queue like the following for example:
> 
>    A(exclusive) -> B(non-exclusive) -> C(non-exclusive)
> 
> Current code will wake up only A when nr_exclusive = 1, but has to wake
> up A, B and C. Make it do as we expect.

You have the list oredered the wrong way around. We add exclusive
waiters to the tail, therefore we'll have woken up all the !exclusive
waiters before we start decrementing nr_exclusive.

Re: [PATCH] sched: Wake up all non-exclusive waiters in __wake_up_common()

[Byungchul Park]

On Mon, Mar 20, 2017 at 11:41:47AM +0100, Peter Zijlstra wrote:
> On Wed, Mar 08, 2017 at 09:21:52AM +0900, Byungchul Park wrote:
> > __wake_up_common() should wake up all non-exclusive waiters and
> > exclusive waiters as many as nr_exclusive, but currently it does not.
> > 
> > Consider a wait queue like the following for example:
> > 
> >    A(exclusive) -> B(non-exclusive) -> C(non-exclusive)
> > 
> > Current code will wake up only A when nr_exclusive = 1, but has to wake
> > up A, B and C. Make it do as we expect.
> 
> You have the list oredered the wrong way around. We add exclusive
> waiters to the tail, therefore we'll have woken up all the !exclusive
> waiters before we start decrementing nr_exclusive.

You're right. I proposed the patch because of __add_wait_queue_exclusive(),
which queues an item at the head. But it was my mistake. Sorry.