linux-kernel - Re: [PATCH] cfq: enable idle for seeky processes on rotational NCQ devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20091003135046.GD31616@kernel.dk>
Date:	Sat, 3 Oct 2009 15:50:46 +0200
From:	Jens Axboe <jens.axboe@...cle.com>
To:	Corrado Zoccolo <czoccolo@...il.com>
Cc:	Linux-Kernel <linux-kernel@...r.kernel.org>,
	Jeff Moyer <jmoyer@...hat.com>,
	Vivek Goyal <vgoyal@...hat.com>, Mike Galbraith <efault@....de>
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 <czoccolo@...il.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/