On Wed, May 21, 2014 at 02:50:00PM -0700, Andrew Morton wrote: > On Wed, 21 May 2014 23:33:54 +0200 Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote: > Alternative solution is not to merge the patch ;) There is always that.. :-) > > Yeah, so we only clear that bit when at 'unlock' we find there are no > > more pending waiters, so if the last unlock still had a waiter, we'll > > leave the bit set. > > Confused. If the last unlock had a waiter, that waiter will get woken > up so there are no waiters any more, so the last unlock clears the flag. > > um, how do we determine that there are no more waiters? By looking at > the waitqueue. But that waitqueue is hashed, so it may contain waiters > for other pages so we're screwed? But we could just go and wake up the > other-page waiters anyway and still clear PG_waiters? > > um2, we're using exclusive waitqueues so we can't (or don't) wake all > waiters, so we're screwed again? Ah, so leave it set. Then when we do an uncontended wakeup, that is a wakeup where there are _no_ waiters left, we'll iterate the entire hashed queue, looking for a matching page. We'll find none, and only then clear the bit. > (This process is proving to be a hard way of writing Mel's changelog btw). Agreed :/ > 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. > If there *are* a lot of collisions, I bet it's because a great pile of > threads are all waiting on the same page. If they're trying to lock > that page then wake-all is bad. But if they're just waiting for IO > completion (probable) then it's OK. Yeah, I'm not entirely sure on the rationale for adding PG_waiters to writeback completion, and yes PG_writeback is a wake-all.
Attachment:
pgpCUIDZn7_YZ.pgp
Description: PGP signature