On Wed, Sep 28, 2016 at 4:11 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> -void unlock_page(struct page *page)
> +void __unlock_page(struct page *page)
> {
> + struct wait_bit_key key = __WAIT_BIT_KEY_INITIALIZER(&page->flags, PG_locked);
> + wait_queue_head_t *wq = page_waitqueue(page);
> +
> + if (waitqueue_active(wq))
> + __wake_up(wq, TASK_NORMAL, 1, &key);
> + else
> + ClearPageContended(page);
> }
> +EXPORT_SYMBOL(__unlock_page);
I think the above needs to be protected. Something like
spin_lock_irqsave(&q->lock, flags);
if (waitqueue_active(wq))
__wake_up_locked(wq, TASK_NORMAL, 1, &key);
else
ClearPageContended(page);
spin_unlock_irqrestore(&q->lock, flags);
because otherwise a new waiter could come in and add itself to the
wait-queue, and then set the bit, and now we clear it (because we
didn't see the new waiter).
The *waiter* doesn't need any extra locking, because doing
add_wait_queue(..);
SetPageContended(page);
is not racy (the add_wait_queue() will now already guarantee that
nobody else clears the bit).
Hmm?
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>