Messages in this thread | | | Date | Tue, 27 Jan 2009 21:05:44 +0100 | From | Oleg Nesterov <> | Subject | Re: [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.
| |