Re: [PATCH] cfq: Make use of service count to estimate the rb_key offset

[Corrado Zoccolo <czoccolo@gmail.com>]

On Mon, Nov 30, 2009 at 4:36 PM, Vivek Goyal <vgoyal@redhat.com> wrote:
> Hi Corrado,
>
> currently rb_key seems to be combination of two things. busy_queues and
> jiffies.
>
> In new scheme, where we decide the share of a workload and then switch to
> new workload, dependence on busy_queues does not seem to make much sense.
>
> Assume, a bunch of sequential readers get backlogged and then few random
> readers gets backlogged. Now random reader will get higher rb_key because
> there are 8 sequential reders on sync-idle tree.
Even if we didn't have busy_queues, we would have the same situation,
e.g. we have the 8 seq readers at time t (jiffies), and the seeky
readers at time t+1.
busy_queues really doesn't add anything when all queues have the same priority.
>
> IIUC, with above logic, even if we expire the sync-idle workload duration
> once, we might not switch to sync-noidle workload and start running the
> sync-idle workload again. (Because minimum slice length restrictions or
> if low_latency is not set).
Yes.
>
> So instead of relying on rb_keys to switch the workload type, why not do
> it in round robin manner across the workload types? So rb_key will be
> significant only with-in service tree and not across service tree?
This is a good option. I have also tested it, and it works quite well
(you can even have an async penalization like in deadline, so you do
few rounds between seq and seeky, and then one of async). Besides a
more complex code, I felt it was against the spirit of CFQ, since in
that way, you are not providing fairness across workloads (especially
if you don't want low_latency).

BTW, my idea how to improve the rb_key computation is:
* for NCQ SSD (or when sched_idle = 0):
  rb_key = jiffies - function(priority)
* for others:
  rb_key = jiffies - sched_resid

Currently, sched_resid is meaningless for NCQ SSD, since we always
expire the queue immediately. Subtracting sched_resid would just give
an advantage to a queue that already dispatched over the ones that
didn't.
Priority, instead, should be used only for NCQ SSD. For the others,
priority already affects time slice, so having it here would cause
over-prioritization.

Thanks,
Corrado

> Thanks
> Vivek