Re: [PATCH] WAIT_BIT_QUEUE

[Oleg Nesterov <oleg@tv-sign.ru>]

Andrew Morton wrote:
>
> Oleg Nesterov <oleg@tv-sign.ru> wrote:
> > 
> >  process waiting in wait_on_page_bit() will be woken only after
> >  the required bit is cleared.
> > 
> >  so there is no need to recheck the bit in do/while loop, because
> >  there is no false wakeups now.
> 
> yup.  Please see the new patches in 2.6.6-mm1 - the waiter puts the bit
> number into the waitqueue structure and the waker tests it before
> delivering the wakeup.

and it puts page or buffer_head in waitqueue instead of just flags.

> +static int page_wake_function(wait_queue_t *wait, unsigned mode, int sync, void *key)
> +{
> +	struct page *page = key;
> +	struct page_wait_queue *wq;
> +
> +	wq = container_of(wait, struct page_wait_queue, wait);
> +	if (wq->page != page || test_bit(wq->bit, &page->flags))
> +		return 0;
> +	else
> +		return autoremove_wake_function(wait, mode, sync, NULL);
> +}

Why bother to check if (wq->page != page) ?
Yes, without this check waiting process can be waken _before_
wake_up_all(page_waitqueue(page)) but i see no problems here.

In fact, this can happen with clean kernel as well.
Let us suppose page_waitqueue(A) == page_waitqueue(B), and
we have two concurrent unlock_page() on these pages.

unlock_page(A)                       unlock_page(B)
TestClearPageLocked(A)
                                     TestClearPageLocked(B)
                                     wake_up_all(page_waitqueue(B)
                                           wakes up process waiting for A,
                                           it returns from wait_on_page_bit()
                                           because !test_bit(bit_nr, &page->flags)
wake_up_all(page_waitqueue(A)
     waiter already running.


So, I beleive, we need not key parameter in page_wake_function.
If we will put flags in waitqueue, bh_wake_function becomes identical
to page_wake_function, and we do not have to modify wakers at all,
there is no need to push page/buffer_head to wake_up().

So, new key parameter for wake_up becomes unneeded.

Oleg.