Re: [PATCH 5/6] xfs: convert the xfsaild threads to a workqueue

[Dave Chinner <david@fromorbit.com>]

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@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs