linux-kernel - Re: [PATCH] cfq-iosched: fix queue depth detection

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: <48AE9CDE.9090504@gelato.unsw.edu.au>
Date:	Fri, 22 Aug 2008 21:02:54 +1000
From:	Aaron Carroll <aaronc@...ato.unsw.edu.au>
To:	Jens Axboe <jens.axboe@...cle.com>
CC:	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] cfq-iosched: fix queue depth detection

Jens Axboe wrote:
> On Fri, Aug 22 2008, Aaron Carroll wrote:
>> Hi Jens,
>>
>> This patch fixes a bug in the hw_tag detection logic causing a huge 
>> performance
>> hit under certain workloads on real queuing devices.  For example, an FIO 
>> load
>> of 16k direct random reads on an 8-disk hardware RAID yields about 2 MiB/s 
>> on
>> default CFQ, while noop achieves over 20 MiB/s.
>>
>> While the solution is pretty ugly, it does have the advantage of adapting to
>> queue depth changes.  Such a situation might occur if the queue depth is
>> configured in userspace late in the boot process.
> 
> I don't think it's that ugly, and I prefer this logic to the existing
> one in fact. Since it's a static property of the device, why did you
> change it to toggle the flag back and forth instead of just setting it
> once?

Because it is possible (albeit uncommon) that the queue depth can change
at run time, like the example I gave.  However, there should be no false
positives; the flag should only be toggled if the queue depth does change.
So even if it doesn't occur often, we can handle this corner case for very
little cost.

> doesn't do queueing. So the interesting window is the one where we have
> more requests pending yet the driver doesn't ask for it. I'd prefer a
> patch that took that more into account, instead of just looking at the
> past 50 samples and then toggle the hw_tag flag depending on the
> behaviour in that time frame. You could easily have a depth of 1 there
> always if it's a sync workload, even if hardware can do tagged queuing.

That's exactly what the lines

	if (cfqd->rq_queued <= CFQ_HW_QUEUE_MIN &&
	    cfqd->rq_in_driver <= CFQ_HW_QUEUE_MIN)
		return;

are for.  It's not just the past 50 samples, but rather 50 samples with
sufficient load to see whether the device could be queuing.


Thanks,
   -- Aaron

--
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/