2009-10-04 16:39:06

by Corrado Zoccolo

[permalink] [raw]
Subject: [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices

Disabled idle window cause large latencies when seeky processes are competing
with async writes, for rotational NCQ devices.

This patch, based on Vivek Goyal's original idea (http://lkml.org/lkml/2009/7/12/110),
unconditionally enables idle window for seeky processes on rotational NCQ devices.
As for non-NCQ devices, a smaller idle window (2ms) is used
for seeky processes compared to normal I/O (8ms).

RAIDs should be marked as non-rotational as well (and probably a better flag
name should be devised), since they can carry multiple operations in parallel.

Signed-off-by: Corrado Zoccolo <[email protected]>
---
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index ebab60c..576e92d 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -1981,10 +1981,14 @@ cfq_update_idle_window(struct cfq_data *cfqd, struct cfq_queue *cfqq,
enable_idle = old_idle = cfq_cfqq_idle_window(cfqq);

if (!atomic_read(&cic->ioc->nr_tasks) || !cfqd->cfq_slice_idle ||
- (!cfqd->cfq_latency && cfqd->hw_tag && CIC_SEEKY(cic)))
+ (blk_queue_nonrot(cfqd->queue) && cfqd->hw_tag && CIC_SEEKY(cic)))
enable_idle = 0;
else if (sample_valid(cic->ttime_samples)) {
- if (cic->ttime_mean > cfqd->cfq_slice_idle)
+ unsigned idle_time = cfqd->cfq_slice_idle;
+ if (CIC_SEEKY(cic))
+ idle_time = CFQ_MIN_TT;
+
+ if (cic->ttime_mean > idle_time)
enable_idle = 0;
else
enable_idle = 1;


2009-10-04 17:37:15

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices

On Sun, Oct 04 2009, Corrado Zoccolo wrote:
> Disabled idle window cause large latencies when seeky processes are competing
> with async writes, for rotational NCQ devices.
>
> This patch, based on Vivek Goyal's original idea (http://lkml.org/lkml/2009/7/12/110),
> unconditionally enables idle window for seeky processes on rotational NCQ devices.
> As for non-NCQ devices, a smaller idle window (2ms) is used
> for seeky processes compared to normal I/O (8ms).
>
> RAIDs should be marked as non-rotational as well (and probably a better flag
> name should be devised), since they can carry multiple operations in parallel.

I think this one is a bit problematic. What I'd like seeky processes to
do is enable 'idle unless other sync read comes in' for such cases,
otherwise it will cost us a lot of performance on the seeky vs seeky
cases because we don't get to take advantage of queuing. It would be
perfectly fine to continue waiting if just async IO comes in, but if we
have other seeky readers then they should get a turn.

I realize that this does skew potential priority issues.

--
Jens Axboe

2009-10-04 18:30:12

by Corrado Zoccolo

[permalink] [raw]
Subject: Re: [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices

On Sun, Oct 4, 2009 at 7:36 PM, Jens Axboe <[email protected]> wrote:
> I think this one is a bit problematic. What I'd like seeky processes to
> do is enable 'idle unless other sync read comes in' for such cases,
> otherwise it will cost us a lot of performance on the seeky vs seeky
> cases because we don't get to take advantage of queuing.

Are we sure that queuing is beneficial in this workload, on non-raid
rotational devices?
If the seeks are still quite local (e.g. when accessing a single
file), given that seek time is proportional to seek length, idling
should provide higher throughput.

Anyway, I'm working on an other patch that will group together all
seeky queues and dispatch them without idling, and idle only on the
last one, so if you prefer, this can be postponed until the other
patch is ready.

Thanks,
Corrado

> It would be
> perfectly fine to continue waiting if just async IO comes in, but if we
> have other seeky readers then they should get a turn.
>
> I realize that this does skew potential priority issues.
>
> --
> Jens Axboe
>

2009-10-04 18:40:23

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices

On Sun, Oct 04 2009, Corrado Zoccolo wrote:
> On Sun, Oct 4, 2009 at 7:36 PM, Jens Axboe <[email protected]> wrote:
> > I think this one is a bit problematic. What I'd like seeky processes to
> > do is enable 'idle unless other sync read comes in' for such cases,
> > otherwise it will cost us a lot of performance on the seeky vs seeky
> > cases because we don't get to take advantage of queuing.
>
> Are we sure that queuing is beneficial in this workload, on non-raid
> rotational devices?
> If the seeks are still quite local (e.g. when accessing a single
> file), given that seek time is proportional to seek length, idling
> should provide higher throughput.

Yes very sure, seek time is only approximately proportional to seek
length. With queuing, you potentially can account for rotational delay,
which is an equally big factor in IO latency. For small seeks, it's easy
the dominating factor even.

> Anyway, I'm working on an other patch that will group together all
> seeky queues and dispatch them without idling, and idle only on the
> last one, so if you prefer, this can be postponed until the other
> patch is ready.

I think so.

--
Jens Axboe