Re: [PATCH 0/1] Correct sorting problem in cfq_service_tree_add

[Corrado Zoccolo <czoccolo@gmail.com>]

On Tue, Nov 24, 2009 at 2:11 PM, Alan D. Brunelle <Alan.Brunelle@hp.com> wrote:
> Found this whilst reviewing the CFQ I/O scheduler code: Currently, this
> routine only sorts using the I/O priority class - it does not properly
> sort prioritized queues within a specific class. The patch changes the
> sort to utilize the full I/O priority (class & priority).

This changes mixes the interpretation of classes and levels within class.
In the original code, those different things have different meanings:
* priority class decides who can use the disk
* priority level within a class determines how much of the disk time
each queue will obtain
In your case. instead, you completely remove the second meaning, and
provide a larger number of levels to just decide the first.

>
> A simple test shows the problem & fixed results: on a 16-way box, for
> each of 12 attached disks I started up 17 processes (one process at each
> possible class/priority). Each process operated on a separate file in
> the file system. I then did two types of tests: (a) direct/synchronous
> and (b) direct/asynchronous w/ an 80/20 read/write split.
>
> I then tabulated the overall I/O performed per task: (first column is
> priority class (1==RT, 2==BE, 3==IDLE), second column is the I/O
> priority (0==highest), then two groupings of read/write data moved
> (total KiBs over a span of 120 seconds):
>
> Synchronous:
>         2.6.32-rc8     2.6.32-rc8+patch
>        Read    Write     Read    Write
>     ----------------   ----------------
> 1 0 |  311164  310760 |  424260  424116 |
> 1 1 |  129712  129792 |  390208  393232 |
> 1 2 |   72312   71284 |     448     420 |
> 1 3 |   40364   41052 |      28      20 |
> 1 4 |   26788   26352 |      28      24 |
> 1 5 |   16936   16940 |      52      32 |
> 1 6 |   11196   11140 |      28      20 |
> 1 7 |    6476    6648 |      20      28 |

The numbers for the patched kernel are bad.
All priority levels > 2 are starved. They can complete an amount of
I/O comparable with lower priority class:
> 2 0 |      24      24 |      40       8 |
> 2 1 |      24      24 |      12      36 |
> 2 2 |      20      28 |      20      28 |
> 2 3 |      28      20 |      24      24 |
> 2 4 |      28      20 |      28      20 |
> 2 5 |      28      20 |      20      28 |
> 2 6 |      24      24 |      20      28 |
> 2 7 |      24      24 |      36      12 |
>
> 3   |      36      12 |      28      20 |
>     ----------------   ----------------
> Sum    615184  614164    815300  818096
>
This is not the intended behaviour, and you don't need 14 priority
levels to get only one use the disk.

Cheers,
Corrado