2009-10-03 08:57:27

by Corrado Zoccolo

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

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 <[email protected]>
---
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;

2009-10-03 13:50:44

by Jens Axboe

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

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 <[email protected]>
> ---
> 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