Messages in this thread | | | Date | Thu, 03 May 2007 22:43:20 +1000 | From | Nick Piggin <> | Subject | Re: 2.6.22 -mm merge plans -- vm bugfixes |
| |
Hugh Dickins wrote: > On Thu, 3 May 2007, Nick Piggin wrote: > >>The problem is that lock/unlock_page is expensive on powerpc, and >>if we improve that, we improve more than just the fault handler... >> >>The attached patch gets performance up a bit by avoiding some >>barriers and some cachelines: > > > There's a strong whiff of raciness about this... > but I could very easily be wrong. > > >>Index: linux-2.6/mm/filemap.c >>=================================================================== >>--- linux-2.6.orig/mm/filemap.c 2007-05-02 15:00:26.000000000 +1000 >>+++ linux-2.6/mm/filemap.c 2007-05-03 08:34:32.000000000 +1000 >>@@ -532,11 +532,13 @@ >> */ >> void fastcall unlock_page(struct page *page) >> { >>+ VM_BUG_ON(!PageLocked(page)); >> smp_mb__before_clear_bit(); >>- if (!TestClearPageLocked(page)) >>- BUG(); >>- smp_mb__after_clear_bit(); >>- wake_up_page(page, PG_locked); >>+ ClearPageLocked(page); >>+ if (unlikely(test_bit(PG_waiters, &page->flags))) { >>+ clear_bit(PG_waiters, &page->flags); >>+ wake_up_page(page, PG_locked); >>+ } >> } >> EXPORT_SYMBOL(unlock_page); >> >>@@ -568,6 +570,11 @@ __lock_page (diff -p would tell us!) >> { >> DEFINE_WAIT_BIT(wait, &page->flags, PG_locked); >> >>+ set_bit(PG_waiters, &page->flags); >>+ if (unlikely(!TestSetPageLocked(page))) { > > > What happens if another cpu is coming through __lock_page at the > same time, did its set_bit, now finds PageLocked, and so proceeds > to the __wait_on_bit_lock? But this cpu now clears PG_waiters, > so this task's unlock_page won't wake the other?
You're right, we can't clear the bit here. Doubt it mattered much anyway?
BTW. I also forgot an smp_mb__after_clear_bit() before the wake_up_page above... that barrier is in the slow path as well though, so it shouldn't matter either.
> > >>+ clear_bit(PG_waiters, &page->flags); >>+ return; >>+ } >> __wait_on_bit_lock(page_waitqueue(page), &wait, sync_page, >> TASK_UNINTERRUPTIBLE); >> } > >
-- SUSE Labs, Novell Inc. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |