Re: [PATCH] io stalls

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

Andrea Arcangeli wrote:

>On Thu, Jun 12, 2003 at 01:04:27PM +1000, Nick Piggin wrote:
>
>>
>>Andrea Arcangeli wrote:
>>
>>
>>>On Thu, Jun 12, 2003 at 12:49:46PM +1000, Nick Piggin wrote:
>>>
>>>
>>>>Andrea Arcangeli wrote:
>>>>
>>>>
>>>>>it does nothing w/ _exclusive and w/o the wake_up_nr, that's why I added
>>>>>the wake_up_nr.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>That is pretty pointless as well. You might as well just start
>>>>waking up at the queue full limit, and wake one at a time.
>>>>
>>>>The purpose for batch_requests was I think for devices with a
>>>>very small request size, to reduce context switches.
>>>>
>>>>
>>>batch_requests at least in my tree matters only when each request is
>>>512btyes and you've some thousand of them to compose a 4M queue or so.
>>>To maximize cpu cache usage etc.. I try to wakeup a task every 512bytes
>>>written, but every 32*512bytes written or so. Of course w/o the
>>>wake_up_nr that I added, that wasn't really working w/ the _exlusive
>>>wakeup.
>>>
>>>if you check my tree you'll see that for sequential I/O with 512k in
>>>each request (not 512bytes!) batch_requests is already a noop.
>>>
>>>
>>
>>You are waking up multiple tasks which will each submit
>>1 request. You want to be waking up 1 task which will
>>submit multiple requests - that is how you will save
>>context switches, cpu cache, etc, and that task's requests
>>will have a much better chance of being merged, or at
>>least serviced as a nice batch than unrelated tasks.
>>
>
>for fairness reasons if there are multiple tasks, I want to wake them
>all and let the others be able to eat requests before the first
>allocates all the batch_sectors. So the current code is fine and
>batch_sectors still works fine with multiple tasks queued in the
>waitqueue, it still makes sense to wake more than one of them at the
>same time to improve cpu utilization (regardless they're different
>tasks, for istance we take less frequently the waitqueue spinlocks
>etc..).
>

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.

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).

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.