Re: [PATCH] mm: filemap: Avoid unnecessary barries and waitqueue lookups in unlock_page fastpath v5

From: Andrew Morton
Date: Thu May 22 2014 - 13:47:29 EST


On Thu, 22 May 2014 09:46:43 +0100 Mel Gorman <mgorman@xxxxxxx> 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...

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/