On Sun, Apr 03, 2011 at 06:59:57AM -0400, Christoph Hellwig wrote: > On Sun, Apr 03, 2011 at 10:38:47AM +1000, Dave Chinner wrote: > > FWIW, we already have an implicit delay for frequent callers when > > the AIL is busy - the uninterruptible sleep for sleeps of <= 20ms. > > That was implemented specifically to rate-limit wakeups while the > > xfsaild was busy pushing. This is essentially a different > > implementation of the same mechanism. > > In that cases maybe the minum delay of the delayed work should stay > at 20ms? I've been looking at this a little more - I don't think the work_pending() check is sufficient. The pending bit is cleared before the work function is called, so while the work is executing we can queue up another work. That means when the work completes and decides it needs to back off for a short while, it won't back off because new work has been been queued and the pending bit has been set. Hence I thinkthis needs an external "work running" bit to be set so that the backoff works as expected. That is, when pushing the tail we need to do: ailp->xa_target = threshold_lsn; if (test_and_set_bit(ailp->pushing)) queue_delayed_work(work, 0); When the AIL work function needs to back off for a certain timeout, it just does: queue_delayed_work(work, tout); and when it completes the push, it does: clear_bit(ailp->pushing); So the next tail push requeues the work again. That gets around the need for implicit wakeup rate limiting, allows tail pushing to keep moving the target forwards while the work is running, and doesn't allow tail pushing to impact on a running push. I'll modify it to do this - it should also simplify the code as well. Cheers, Dave. -- Dave Chinner david@xxxxxxxxxxxxx _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
