Re: [RFC v6] wait: prevent exclusive waiter starvation

[Oleg Nesterov <oleg@redhat.com>]

On 01/27, Johannes Weiner wrote:
>
> +void abort_exclusive_wait(wait_queue_head_t *q, wait_queue_t *wait)
> +{
> +	unsigned long flags;
> +
> +	__set_current_state(TASK_RUNNING);
> +	spin_lock_irqsave(&q->lock, flags);
> +	if (list_empty(&wait->task_list))

Hmm... it should be !list_empty() ?

> +		list_del_init(&wait->task_list);
> +	/*
> +	 * If we were woken through the waitqueue (waker removed
> +	 * us from the list) we must ensure the next waiter down
> +	 * the line is woken up.  The callsite will not do it as
> +	 * it didn't finish waiting successfully.
> +	 */
> +	else if (waitqueue_active(q))
> +		__wake_up_locked(q, TASK_INTERRUPTIBLE);
> +	spin_unlock_irqrestore(&q->lock, flags);
> +}

Well, personally I don't care, but this is against CodingStyle rules ;)

>  int autoremove_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key)
>  {
>  	int ret = default_wake_function(wait, mode, sync, key);
> @@ -177,17 +218,19 @@ int __sched
>  __wait_on_bit_lock(wait_queue_head_t *wq, struct wait_bit_queue *q,
>  			int (*action)(void *), unsigned mode)
>  {
> -	int ret = 0;
> -
>  	do {
> +		int ret;
> +
>  		prepare_to_wait_exclusive(wq, &q->wait, mode);
> -		if (test_bit(q->key.bit_nr, q->key.flags)) {
> -			if ((ret = (*action)(q->key.flags)))
> -				break;
> -		}
> +		if (!test_bit(q->key.bit_nr, q->key.flags))
> +			continue;
> +		if (!(ret = action(q->key.flags)))
> +			continue;
> +		abort_exclusive_wait(wq, &q->wait);

No, no. We should use the same key in abort_exclusive_wait().
Otherwise, how can we wakeup the next waiter which needs this
bit in the same page->flags?

That is why I suggested finish_wait_exclusive(..., void *key)
which should we passed to __wake_up_common().

Oleg.