Messages in this thread | | | Date | Thu, 22 May 2014 10:47:22 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] mm: filemap: Avoid unnecessary barries and waitqueue lookups in unlock_page fastpath v5 |
| |
On Thu, 22 May 2014 09:46:43 +0100 Mel Gorman <mgorman@suse.de> wrote:
> > > If I'm still on track here, what happens if we switch to wake-all so we > > > can avoid the dangling flag? I doubt if there are many collisions on > > > that hash table? > > > > Wake-all will be ugly and loose a herd of waiters, all racing to > > acquire, all but one of whoem will loose the race. It also looses the > > fairness, its currently a FIFO queue. Wake-all will allow starvation. > > > > And the cost of the thundering herd of waiters may offset any benefit of > reducing the number of calls to page_waitqueue and waker functions.
Well, none of this has been demonstrated.
As I speculated earlier, hash chain collisions will probably be rare, except for the case where a bunch of processes are waiting on the same page. And in this case, perhaps wake-all is the desired behavior.
Take a look at do_read_cache_page(). It does lock_page(), but it doesn't actually *need* to. It checks ->mapping and PG_uptodate and then... unlocks the page! We could have used wait_on_page_locked() there and permitted concurrent threads to run concurrently.
btw, I'm struggling a bit to understand why we bother checking ->mapping there as we're about to unlock the page anyway...
| |