On Thu, May 22, 2014 at 10:47:22AM -0700, Andrew Morton wrote: > 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. > True, but it's also the type of thing that would deserve a patch of its own with some separation in case bisection fingerpoints to a patch that is doing too much on its own. > As I speculated earlier, hash chain collisions will probably be rare, They are meant to be (well, they're documented to be). It's the primary reason why I'm not concerned about "dangling waiters" being that common a case. > 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. > It does that later when it calls wait_on_page_read but the flow is weird. It looks like the first lock_page was to serialise against any IO and double check it was not racing against a parallel reclaim although the elevated reference count should have prevented that. Historical artifact maybe? It looks like there could be some improvement there but also would deserve a patch on its own. -- Mel Gorman SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
