lkml.org 
[lkml]   [2009]   [Jan]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC v6] wait: prevent exclusive waiter starvation
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.



\
 
 \ /
  Last update: 2009-01-27 21:13    [W:0.126 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site