Re: [PATCH] io stalls

[Chris Mason <mason@suse.com>]

On Wed, 2003-06-11 at 21:04, Nick Piggin wrote:
> Andrea Arcangeli wrote:
> 
> >On Wed, Jun 11, 2003 at 02:27:13PM -0400, Chris Mason wrote:
> >
> >>On Wed, 2003-06-11 at 14:12, Andrea Arcangeli wrote:
> >>
> >>>On Wed, Jun 11, 2003 at 01:42:41PM -0400, Chris Mason wrote:
> >>>
> >>>>+		if (q->rq[rw].count >= q->batch_requests) {
> >>>>+			smp_mb();
> >>>>+			if (waitqueue_active(&q->wait_for_requests[rw]))
> >>>>+				wake_up(&q->wait_for_requests[rw]);
> >>>>
> >>>in my tree I also changed this to:
> >>>
> >>>				wake_up_nr(&q->wait_for_requests[rw], q->rq[rw].count);
> >>>
> >>>otherwise only one waiter will eat the requests, while multiple waiters
> >>>can eat requests in parallel instead because we freed not just 1 request
> >>>but many of them.
> >>>
> >>I tried a few variations of this yesterday and they all led to horrible
> >>latencies, but I couldn't really explain why.  I had a bunch of other
> >>
> >
> >the I/O latency in theory shouldn't change, we're not reordering the
> >queue at all, they'll go to sleep immediatly again if __get_request
> >returns null.
> >
> 
> And go to the end of the queue?
> 

This got dragged into private mail for a few messages, but we figured
out the problem turns into scheduling fairness with wake_up_nr()

32 procs might get woken, but when the first of those procs gets a
request, he'll wake another, and so on.  But there's no promise that
getting woken fairly means you'll get scheduled fairly, so you might not
get scheduled in for quite a while, perhaps even after new requests have
gone onto the wait queue and gotten woken up.

The real problem is get_request_wait_wakeup, andrea is working on a few
changes to that.  

-chris