* [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices
@ 2009-10-04 16:37 Corrado Zoccolo
2009-10-04 17:36 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Corrado Zoccolo @ 2009-10-04 16:37 UTC (permalink / raw)
To: Linux-Kernel, Jens Axboe, Jeff Moyer
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 <czoccolo@gmail.com>
---
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;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices
2009-10-04 16:37 [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices Corrado Zoccolo
@ 2009-10-04 17:36 ` Jens Axboe
2009-10-04 18:29 ` Corrado Zoccolo
0 siblings, 1 reply; 6+ messages in thread
From: Jens Axboe @ 2009-10-04 17:36 UTC (permalink / raw)
To: Corrado Zoccolo; +Cc: Linux-Kernel, Jeff Moyer
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices
2009-10-04 17:36 ` Jens Axboe
@ 2009-10-04 18:29 ` Corrado Zoccolo
2009-10-04 18:39 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Corrado Zoccolo @ 2009-10-04 18:29 UTC (permalink / raw)
To: Jens Axboe; +Cc: Linux-Kernel, Jeff Moyer
On Sun, Oct 4, 2009 at 7:36 PM, Jens Axboe <jens.axboe@oracle.com> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices
2009-10-04 18:29 ` Corrado Zoccolo
@ 2009-10-04 18:39 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2009-10-04 18:39 UTC (permalink / raw)
To: Corrado Zoccolo; +Cc: Linux-Kernel, Jeff Moyer
On Sun, Oct 04 2009, Corrado Zoccolo wrote:
> On Sun, Oct 4, 2009 at 7:36 PM, Jens Axboe <jens.axboe@oracle.com> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices
@ 2009-10-03 8:57 Corrado Zoccolo
2009-10-03 13:50 ` Jens Axboe
0 siblings, 1 reply; 6+ messages in thread
From: Corrado Zoccolo @ 2009-10-03 8:57 UTC (permalink / raw)
To: Linux-Kernel, Jens Axboe, Jeff Moyer; +Cc: Vivek Goyal, Mike Galbraith
Idle window is currently disabled for seeky processes on all NCQ devices.
This is causing 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),
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 <czoccolo@gmail.com>
---
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 1ca813b..7116f11 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -1951,10 +1951,12 @@ 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->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 = CIC_SEEKY(cic) ? CFQ_MIN_TT
+ : cfqd->cfq_slice_idle;
+ if (cic->ttime_mean > idle_time)
enable_idle = 0;
else
enable_idle = 1;
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices
2009-10-03 8:57 Corrado Zoccolo
@ 2009-10-03 13:50 ` Jens Axboe
0 siblings, 0 replies; 6+ messages in thread
From: Jens Axboe @ 2009-10-03 13:50 UTC (permalink / raw)
To: Corrado Zoccolo; +Cc: Linux-Kernel, Jeff Moyer, Vivek Goyal, Mike Galbraith
On Sat, Oct 03 2009, Corrado Zoccolo wrote:
> Idle window is currently disabled for seeky processes on all NCQ devices.
> This is causing 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),
> 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 <czoccolo@gmail.com>
> ---
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 1ca813b..7116f11 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -1951,10 +1951,12 @@ 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->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 = CIC_SEEKY(cic) ? CFQ_MIN_TT
> + : cfqd->cfq_slice_idle;
> + if (cic->ttime_mean > idle_time)
> enable_idle = 0;
> else
> enable_idle = 1;
Please don't use the ?: constructs, they are not very readable
(especially not with multi-lines).
Can you resend this on top of the current for-linus branch, it has a few
cfq patches that cause this not to apply anyway.
--
Jens Axboe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-10-04 18:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-04 16:37 [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices Corrado Zoccolo
2009-10-04 17:36 ` Jens Axboe
2009-10-04 18:29 ` Corrado Zoccolo
2009-10-04 18:39 ` Jens Axboe
-- strict thread matches above, loose matches on Subject: below --
2009-10-03 8:57 Corrado Zoccolo
2009-10-03 13:50 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).