linux-kernel - Re: [PATCH 1/1] CFQ: fix handling 'deep' cfqq

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+X1aCRSADrUB+YVJ9j5u+VgGn=8fKw7A1eVVzPtODg+hK-B-w@mail.gmail.com>
Date:	Tue, 23 Aug 2011 12:56:02 +0400
From:	Maxim Patlasov <maxim.patlasov@...il.com>
To:	Shaohua Li <shli@...nel.org>
Cc:	axboe@...nel.dk, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] CFQ: fix handling 'deep' cfqq

Hi,

>> An aproach suggested here avoids performance degradations mentioned above.
>> With this patch applied, the performance on slow hdd is the same as it used
>> to be before 8e1ac6655104bc6e1e79d67e2df88cc8fa9b6e07, and, on fast h/w-raids,
>> it's roughly the same as for noop scheduler or CFQ with slice_idle=0.
> idle is a usual cause of cfq performance issue. did you have test in
> disk without NCQ?

Yes, on the node with "slow hdd" CFQ detected it as hw_tag=0. I
explained what I tested in cover message (subj: [PATCH 0/1] CFQ:
fixing performance issues).

> And did you test if this will hurt the performance of Vivek's original problem?

No. What's Vivek's original problem?

> snip
>> +       if (cfq_cfqq_deep_early(cfqq) && cfqq->n_dispatched >= CFQQ_DEEP_THR) {
>> +               if (cfqq->first_dispatch == jiffies)
>> +                       cfqd->cfq_disk_looks_fast++;
>> +               else
>> +                       cfqd->cfq_disk_looks_slow++;
>> +
> jiffies is too coarse here. A disk with NCQ can dispatch several
> requests within one jiffy.

If a disk with NCQ dispatches four requests in raw within one jiffy
regularly, the patch I suggested will claim it as "fast enough". It
should be beneficial to disable idling for deep&seeky cfqq in this
case, imho. Anyway, existing code:

>	/*
>	 * This is a deep seek queue, but the device is much faster than
>	 * the queue can deliver, don't idle
>	 **/
>	if (CFQQ_SEEKY(cfqq) && cfq_cfqq_idle_window(cfqq) &&
>	    (cfq_cfqq_slice_new(cfqq) ||
>	    (cfqq->slice_end - jiffies > jiffies - cfqq->slice_start))) {
>		cfq_clear_cfqq_deep(cfqq);
>		cfq_clear_cfqq_idle_window(cfqq);
>	}
>

would surely disable idling in this case. So, the patch I suggested
doesn't make things worse (as compared with existing implementation).

Thanks,
Maxim
--
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/