CFQ idling causes reduced IOPS throughput on non-rotational disks.
Since disk head seeking is not applicable to SSDs, it doesn't really
help performance by anticipating future near-by IO requests.
By turning off idling (and switching to IOPS mode), we allow other
processes to dispatch IO requests down to the driver and so increase IO
throughput.
Following FIO benchmark results were taken on a cloud SSD offering with
idling on and off:
Idling iops avg-lat(ms) stddev bw
------------------------------------------------------
On 7054 90.107 38.697 28217KB/s
Off 29255 21.836 11.730 117022KB/s
fio --name=temp --size=100G --time_based --ioengine=libaio \
--randrepeat=0 --direct=1 --invalidate=1 --verify=0 \
--verify_fatal=0 --rw=randread --blocksize=4k --group_reporting=1 \
--filename=/dev/sdb --runtime=10 --iodepth=64 --numjobs=10
And the following is from a local SSD run:
Idling iops avg-lat(ms) stddev bw
------------------------------------------------------
On 19320 33.043 14.068 77281KB/s
Off 21626 29.465 12.662 86507KB/s
fio --name=temp --size=5G --time_based --ioengine=libaio \
--randrepeat=0 --direct=1 --invalidate=1 --verify=0 \
--verify_fatal=0 --rw=randread --blocksize=4k --group_reporting=1 \
--filename=/fio_data --runtime=10 --iodepth=64 --numjobs=10
Reviewed-by: Nauman Rafique <[email protected]>
Signed-off-by: Tahsin Erdogan <[email protected]>
---
block/cfq-iosched.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 5da8e6e..402be01 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -4460,7 +4460,7 @@ static int cfq_init_queue(struct request_queue *q, struct elevator_type *e)
cfqd->cfq_slice[1] = cfq_slice_sync;
cfqd->cfq_target_latency = cfq_target_latency;
cfqd->cfq_slice_async_rq = cfq_slice_async_rq;
- cfqd->cfq_slice_idle = cfq_slice_idle;
+ cfqd->cfq_slice_idle = blk_queue_nonrot(q) ? 0 : cfq_slice_idle;
cfqd->cfq_group_idle = cfq_group_idle;
cfqd->cfq_latency = 1;
cfqd->hw_tag = -1;
--
2.2.0.rc0.207.ga3a616c
On Wed, May 27, 2015 at 1:14 PM, Tahsin Erdogan <[email protected]> wrote:
> On Tue, May 19, 2015 at 1:55 PM, Tahsin Erdogan <[email protected]> wrote:
>> CFQ idling causes reduced IOPS throughput on non-rotational disks.
>> Since disk head seeking is not applicable to SSDs, it doesn't really
>> help performance by anticipating future near-by IO requests.
>>
>> By turning off idling (and switching to IOPS mode), we allow other
>> processes to dispatch IO requests down to the driver and so increase IO
>> throughput.
>>
>> Following FIO benchmark results were taken on a cloud SSD offering with
>> idling on and off:
>>
>> Idling iops avg-lat(ms) stddev bw
>> ------------------------------------------------------
>> On 7054 90.107 38.697 28217KB/s
>> Off 29255 21.836 11.730 117022KB/s
>>
>> fio --name=temp --size=100G --time_based --ioengine=libaio \
>> --randrepeat=0 --direct=1 --invalidate=1 --verify=0 \
>> --verify_fatal=0 --rw=randread --blocksize=4k --group_reporting=1 \
>> --filename=/dev/sdb --runtime=10 --iodepth=64 --numjobs=10
>>
>> And the following is from a local SSD run:
>>
>> Idling iops avg-lat(ms) stddev bw
>> ------------------------------------------------------
>> On 19320 33.043 14.068 77281KB/s
>> Off 21626 29.465 12.662 86507KB/s
>>
>> fio --name=temp --size=5G --time_based --ioengine=libaio \
>> --randrepeat=0 --direct=1 --invalidate=1 --verify=0 \
>> --verify_fatal=0 --rw=randread --blocksize=4k --group_reporting=1 \
>> --filename=/fio_data --runtime=10 --iodepth=64 --numjobs=10
>>
>> Reviewed-by: Nauman Rafique <[email protected]>
>> Signed-off-by: Tahsin Erdogan <[email protected]>
>> ---
>> block/cfq-iosched.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
>> index 5da8e6e..402be01 100644
>> --- a/block/cfq-iosched.c
>> +++ b/block/cfq-iosched.c
>> @@ -4460,7 +4460,7 @@ static int cfq_init_queue(struct request_queue *q, struct elevator_type *e)
>> cfqd->cfq_slice[1] = cfq_slice_sync;
>> cfqd->cfq_target_latency = cfq_target_latency;
>> cfqd->cfq_slice_async_rq = cfq_slice_async_rq;
>> - cfqd->cfq_slice_idle = cfq_slice_idle;
>> + cfqd->cfq_slice_idle = blk_queue_nonrot(q) ? 0 : cfq_slice_idle;
>> cfqd->cfq_group_idle = cfq_group_idle;
>> cfqd->cfq_latency = 1;
>> cfqd->hw_tag = -1;
>> --
>> 2.2.0.rc0.207.ga3a616c
>>
>
> Ping...
Trying once more..
On 06/05/2015 04:58 PM, Tahsin Erdogan wrote:
> On Wed, May 27, 2015 at 1:14 PM, Tahsin Erdogan <[email protected]> wrote:
>> On Tue, May 19, 2015 at 1:55 PM, Tahsin Erdogan <[email protected]> wrote:
>>> CFQ idling causes reduced IOPS throughput on non-rotational disks.
>>> Since disk head seeking is not applicable to SSDs, it doesn't really
>>> help performance by anticipating future near-by IO requests.
>>>
>>> By turning off idling (and switching to IOPS mode), we allow other
>>> processes to dispatch IO requests down to the driver and so increase IO
>>> throughput.
>>>
>>> Following FIO benchmark results were taken on a cloud SSD offering with
>>> idling on and off:
>>>
>>> Idling iops avg-lat(ms) stddev bw
>>> ------------------------------------------------------
>>> On 7054 90.107 38.697 28217KB/s
>>> Off 29255 21.836 11.730 117022KB/s
>>>
>>> fio --name=temp --size=100G --time_based --ioengine=libaio \
>>> --randrepeat=0 --direct=1 --invalidate=1 --verify=0 \
>>> --verify_fatal=0 --rw=randread --blocksize=4k --group_reporting=1 \
>>> --filename=/dev/sdb --runtime=10 --iodepth=64 --numjobs=10
>>>
>>> And the following is from a local SSD run:
>>>
>>> Idling iops avg-lat(ms) stddev bw
>>> ------------------------------------------------------
>>> On 19320 33.043 14.068 77281KB/s
>>> Off 21626 29.465 12.662 86507KB/s
>>>
>>> fio --name=temp --size=5G --time_based --ioengine=libaio \
>>> --randrepeat=0 --direct=1 --invalidate=1 --verify=0 \
>>> --verify_fatal=0 --rw=randread --blocksize=4k --group_reporting=1 \
>>> --filename=/fio_data --runtime=10 --iodepth=64 --numjobs=10
>>>
>>> Reviewed-by: Nauman Rafique <[email protected]>
>>> Signed-off-by: Tahsin Erdogan <[email protected]>
>>> ---
>>> block/cfq-iosched.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
>>> index 5da8e6e..402be01 100644
>>> --- a/block/cfq-iosched.c
>>> +++ b/block/cfq-iosched.c
>>> @@ -4460,7 +4460,7 @@ static int cfq_init_queue(struct request_queue *q, struct elevator_type *e)
>>> cfqd->cfq_slice[1] = cfq_slice_sync;
>>> cfqd->cfq_target_latency = cfq_target_latency;
>>> cfqd->cfq_slice_async_rq = cfq_slice_async_rq;
>>> - cfqd->cfq_slice_idle = cfq_slice_idle;
>>> + cfqd->cfq_slice_idle = blk_queue_nonrot(q) ? 0 : cfq_slice_idle;
>>> cfqd->cfq_group_idle = cfq_group_idle;
>>> cfqd->cfq_latency = 1;
>>> cfqd->hw_tag = -1;
>>> --
>>> 2.2.0.rc0.207.ga3a616c
>>>
>>
>> Ping...
>
> Trying once more..
This one worked :-). I agree, it's probably the sane thing to do, I'll
apply this for 4.2.
--
Jens Axboe
Hi,
On Tue, May 19, 2015 at 01:55:21PM -0700, Tahsin Erdogan wrote:
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -4460,7 +4460,7 @@ static int cfq_init_queue(struct request_queue *q, struct elevator_type *e)
> cfqd->cfq_slice[1] = cfq_slice_sync;
> cfqd->cfq_target_latency = cfq_target_latency;
> cfqd->cfq_slice_async_rq = cfq_slice_async_rq;
> - cfqd->cfq_slice_idle = cfq_slice_idle;
> + cfqd->cfq_slice_idle = blk_queue_nonrot(q) ? 0 : cfq_slice_idle;
> cfqd->cfq_group_idle = cfq_group_idle;
> cfqd->cfq_latency = 1;
> cfqd->hw_tag = -1;
Did you test this patch with regular AHCI SSD devices? Applying it on
top of v4.1-rc7 makes no difference, slice_idle is still initialized to
8 in my setup, while rotational is 0.
Isn't the elevator initialized long before the non-rotational flag is
actually set on the device (which probably happens after it's probed on
the scsi bus)?
On 06/09/2015 04:18 AM, Romain Francoise wrote:
> Hi,
>
> On Tue, May 19, 2015 at 01:55:21PM -0700, Tahsin Erdogan wrote:
>> --- a/block/cfq-iosched.c
>> +++ b/block/cfq-iosched.c
>> @@ -4460,7 +4460,7 @@ static int cfq_init_queue(struct request_queue *q, struct elevator_type *e)
>> cfqd->cfq_slice[1] = cfq_slice_sync;
>> cfqd->cfq_target_latency = cfq_target_latency;
>> cfqd->cfq_slice_async_rq = cfq_slice_async_rq;
>> - cfqd->cfq_slice_idle = cfq_slice_idle;
>> + cfqd->cfq_slice_idle = blk_queue_nonrot(q) ? 0 : cfq_slice_idle;
>> cfqd->cfq_group_idle = cfq_group_idle;
>> cfqd->cfq_latency = 1;
>> cfqd->hw_tag = -1;
>
> Did you test this patch with regular AHCI SSD devices? Applying it on
> top of v4.1-rc7 makes no difference, slice_idle is still initialized to
> 8 in my setup, while rotational is 0.
>
> Isn't the elevator initialized long before the non-rotational flag is
> actually set on the device (which probably happens after it's probed on
> the scsi bus)?
You are absolutely correct. What happens is that the queue is allocated
and initialized, and cfq checks the flag. But the flag is set later in
the process, when we have finished probing the device checked if it's
rotational or not.
There are a few options to handle this. The attached might work, not
tested at all. Basically it adds an io sched registration hook, that is
called when we are adding the disk on the queue. Non-rotational
detection should be done at that point.
Does that work for you?
--
Jens Axboe
Thanks for catching this.
In my testing, I was switching to cfq through sysfs. Since disk
initialization happens earlier than manual switching, I didn't hit
this problem.
On Tue, Jun 9, 2015 at 10:42 AM, Jens Axboe <[email protected]> wrote:
> On 06/09/2015 04:18 AM, Romain Francoise wrote:
>>
>> Hi,
>>
>> On Tue, May 19, 2015 at 01:55:21PM -0700, Tahsin Erdogan wrote:
>>>
>>> --- a/block/cfq-iosched.c
>>> +++ b/block/cfq-iosched.c
>>> @@ -4460,7 +4460,7 @@ static int cfq_init_queue(struct request_queue *q,
>>> struct elevator_type *e)
>>> cfqd->cfq_slice[1] = cfq_slice_sync;
>>> cfqd->cfq_target_latency = cfq_target_latency;
>>> cfqd->cfq_slice_async_rq = cfq_slice_async_rq;
>>> - cfqd->cfq_slice_idle = cfq_slice_idle;
>>> + cfqd->cfq_slice_idle = blk_queue_nonrot(q) ? 0 : cfq_slice_idle;
>>> cfqd->cfq_group_idle = cfq_group_idle;
>>> cfqd->cfq_latency = 1;
>>> cfqd->hw_tag = -1;
>>
>>
>> Did you test this patch with regular AHCI SSD devices? Applying it on
>> top of v4.1-rc7 makes no difference, slice_idle is still initialized to
>> 8 in my setup, while rotational is 0.
>>
>> Isn't the elevator initialized long before the non-rotational flag is
>> actually set on the device (which probably happens after it's probed on
>> the scsi bus)?
>
>
> You are absolutely correct. What happens is that the queue is allocated and
> initialized, and cfq checks the flag. But the flag is set later in the
> process, when we have finished probing the device checked if it's rotational
> or not.
>
> There are a few options to handle this. The attached might work, not tested
> at all. Basically it adds an io sched registration hook, that is called when
> we are adding the disk on the queue. Non-rotational detection should be done
> at that point.
>
> Does that work for you?
>
> --
> Jens Axboe
>
Hi,
On Tue, Jun 09, 2015 at 11:42:29AM -0600, Jens Axboe wrote:
> There are a few options to handle this. The attached might work, not
> tested at all. Basically it adds an io sched registration hook, that is
> called when we are adding the disk on the queue. Non-rotational
> detection should be done at that point.
>
> Does that work for you?
Yep, that works perfectly in my (admittedly limited) testing; slice_idle
is correctly set to 0 on non-rotational devices and keeps its default
value of 8 otherwise. Feel free to add my Tested-by.
Thanks!
On 06/10/2015 12:44 AM, Romain Francoise wrote:
> Hi,
>
> On Tue, Jun 09, 2015 at 11:42:29AM -0600, Jens Axboe wrote:
>> There are a few options to handle this. The attached might work, not
>> tested at all. Basically it adds an io sched registration hook, that is
>> called when we are adding the disk on the queue. Non-rotational
>> detection should be done at that point.
>>
>> Does that work for you?
>
> Yep, that works perfectly in my (admittedly limited) testing; slice_idle
> is correctly set to 0 on non-rotational devices and keeps its default
> value of 8 otherwise. Feel free to add my Tested-by.
>
> Thanks!
Thanks for testing, it is now committed.
--
Jens Axboe