| Date | Tue, 13 May 2014 13:53:13 +0100 | From | Mel Gorman <> | Subject | Re: [PATCH 19/19] mm: filemap: Avoid unnecessary barries and waitqueue lookups in unlock_page fastpath |
| |
On Tue, May 13, 2014 at 10:45:50AM +0100, Mel Gorman wrote: > void unlock_page(struct page *page) > { > + wait_queue_head_t *wqh = clear_page_waiters(page); > + > VM_BUG_ON_PAGE(!PageLocked(page), page); > + > + /* > + * No additional barrier needed due to clear_bit_unlock barriering all updates > + * before waking waiters > + */ > clear_bit_unlock(PG_locked, &page->flags); > - smp_mb__after_clear_bit(); > - wake_up_page(page, PG_locked);
This is wrong. The smp_mb__after_clear_bit() is still required to ensure that the cleared bit is visible before the wakeup on all architectures.
---8<--- diff --git a/mm/filemap.c b/mm/filemap.c index 6ac066e..028b5a1 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -819,11 +819,8 @@ void unlock_page(struct page *page) VM_BUG_ON_PAGE(!PageLocked(page), page); - /* - * No additional barrier needed due to clear_bit_unlock barriering all updates - * before waking waiters - */ clear_bit_unlock(PG_locked, &page->flags); + smp_mb__after_clear_bit(); /* * Wake the queue if waiters were detected. Ordinarily this wakeup
|