Re: [PATCH] io stalls

[Nick Piggin <piggin@cyberone.com.au>]

Andrea Arcangeli wrote:

>On Thu, Jun 12, 2003 at 01:20:44PM +1000, Nick Piggin wrote:
>
>>Its no less fair this way, tasks will still be woken in fifo
>>order. They will just be given the chance to submit a batch
>>of requests.
>>
>
>If you change the behaviour with queued_task_nr > batch_requests it is
>less fair period. Whatever else thing I don't care about right now
>because it is a minor cpu improvement anyways.
>
>I'm not talking about performance, I'm talking about latency and
>fariness only. This is the whole point of the ->full logic.
>

I say each task getting 8 requests at a time is as fair
as each getting 1 request at a time. Yes, you may get a
worse latency, but _fairness_ is the same.

>
>>I think the cpu utilization gain of waking a number of tasks
>>at once would be outweighed by advantage of waking 1 task
>>and not putting it to sleep again for a number of requests.
>>You obviously are not claiming concurrency improvements, as
>>your method would also increase contention on the io lock
>>(or the queue lock in 2.5).
>>
>
>I'm claiming that with queued_task_nr > batch_requests the
>batch_requests logic still has a chance to save some cpu, this is the
>only reason I didn't nuke it completely as you suggested some email ago.
>

Well I'm not so sure that your method will do a great deal
of good. On SMP you would increase contention on the spinlock.
IMO it would be better to serialise them on the waitqueue
instead of a spinlock seeing as they are already sleeping.

>
>>Then you have the cache gains of running each task for a
>>longer period of time. You also get possible IO scheduling
>>improvements.
>>
>>Consider 8 requests, batch_requests at 4, 10 tasks writing
>>to different areas of disk.
>>
>>Your method still only allows each task to have 1 request in
>>the elevator at once. Mine allows each to have a run of 4
>>requests in the elevator.
>>
>
>I definitely want 1 request in the elevator at once or we can as well
>drop your ->full and return to be unfair. The whole point of ->full is
>to get the total fariness, across the tasks in the queue queue, and for
>tasks outside the queue calling get_request too. Since not all tasks
>will fit in the I/O queue, providing a very fair FIFO in the
>wait_for_request is fundamental to provide any sort of latency
>guarantee IMHO (the fact an _exclusive wakeup removal that mixes stuff
>and probably has the side effect of being more fair, made that much
>difference to mainline users kind of confirms that).
>
>
I think we'll just have to agree to disagree here. This
sort of thing came up in our CFQ discussion as well. Its
not that I think your way is without merits, but I think
in an overload situtation its a better aim to attempt to
keep throughput up rather than attempt to provide the
lowest possible latency.