Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932326AbbFISFy (ORCPT ); Tue, 9 Jun 2015 14:05:54 -0400 Received: from mail-pa0-f41.google.com ([209.85.220.41]:34474 "EHLO mail-pa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752262AbbFISFr (ORCPT ); Tue, 9 Jun 2015 14:05:47 -0400 Message-ID: <55772585.30707@kernel.dk> Date: Tue, 09 Jun 2015 11:42:29 -0600 From: Jens Axboe User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Romain Francoise , Tahsin Erdogan CC: linux-kernel@vger.kernel.org Subject: Re: [PATCH] block: Make CFQ default to IOPS mode on SSDs References: <1432068921-17184-1-git-send-email-tahsin@google.com> <873821kww7.fsf@orebokech.com> In-Reply-To: <873821kww7.fsf@orebokech.com> Content-Type: multipart/mixed; boundary="------------000004090506080704020705" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4471 Lines: 129 This is a multi-part message in MIME format. --------------000004090506080704020705 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit 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 --------------000004090506080704020705 Content-Type: text/x-patch; name="elv-register.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="elv-register.patch" diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index c808ad87652d..af8918eb7cd5 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -4508,7 +4508,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 = blk_queue_nonrot(q) ? 0 : cfq_slice_idle; + cfqd->cfq_slice_idle = cfq_slice_idle; cfqd->cfq_group_idle = cfq_group_idle; cfqd->cfq_latency = 1; cfqd->hw_tag = -1; @@ -4525,6 +4525,15 @@ out_free: return ret; } +static void cfq_registered_queue(struct request_queue *q) +{ + struct elevator_queue *e = q->elevator; + struct cfq_data *cfqd = e->elevator_data; + + if (blk_queue_nonrot(q)) + cfqd->cfq_slice_idle = 0; +} + /* * sysfs parts below --> */ @@ -4640,6 +4649,7 @@ static struct elevator_type iosched_cfq = { .elevator_may_queue_fn = cfq_may_queue, .elevator_init_fn = cfq_init_queue, .elevator_exit_fn = cfq_exit_queue, + .elevator_registered_fn = cfq_registered_queue, }, .icq_size = sizeof(struct cfq_io_cq), .icq_align = __alignof__(struct cfq_io_cq), diff --git a/block/elevator.c b/block/elevator.c index 59794d0d38e3..5f0452734a40 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -810,6 +810,8 @@ int elv_register_queue(struct request_queue *q) } kobject_uevent(&e->kobj, KOBJ_ADD); e->registered = 1; + if (e->type->ops.elevator_registered_fn) + e->type->ops.elevator_registered_fn(q); } return error; } diff --git a/include/linux/elevator.h b/include/linux/elevator.h index 45a91474487d..638b324f0291 100644 --- a/include/linux/elevator.h +++ b/include/linux/elevator.h @@ -39,6 +39,7 @@ typedef void (elevator_deactivate_req_fn) (struct request_queue *, struct reques typedef int (elevator_init_fn) (struct request_queue *, struct elevator_type *e); typedef void (elevator_exit_fn) (struct elevator_queue *); +typedef void (elevator_registered_fn) (struct request_queue *); struct elevator_ops { @@ -68,6 +69,7 @@ struct elevator_ops elevator_init_fn *elevator_init_fn; elevator_exit_fn *elevator_exit_fn; + elevator_registered_fn *elevator_registered_fn; }; #define ELV_NAME_MAX (16) --------------000004090506080704020705-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/