From 23a3965c19616da0b723f0a83288dfc76aace016 Mon Sep 17 00:00:00 2001 From: Paolo Valente Date: Sat, 4 Feb 2017 17:31:41 +0100 Subject: [PATCH 01/10] Remove wrong compilation warning BUGFIX: Removed a wrong compilation warning, due to the compiler not taking into account short circuit in a condition. Signed-off-by: Paolo Valente --- block/bfq-sched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/bfq-sched.c b/block/bfq-sched.c index 797bce7..2e9dc59 100644 --- a/block/bfq-sched.c +++ b/block/bfq-sched.c @@ -1301,7 +1301,7 @@ static void bfq_deactivate_entity(struct bfq_entity *entity, bool expiration) { struct bfq_sched_data *sd; - struct bfq_entity *parent; + struct bfq_entity *parent = NULL; for_each_entity_safe(entity, parent) { sd = entity->sched_data; From 62fcb1a9aaa77eb835640d8681fd2f5ad0f1111a Mon Sep 17 00:00:00 2001 From: Paolo Valente Date: Mon, 19 Dec 2016 17:14:58 +0100 Subject: [PATCH 02/10] Add a ton of forgotten static qualifiers BUGIFX: Added several forgotten static qualifiers in function definitions (completely harmless issue). Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 2a2c130..98a1acd 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -107,7 +107,7 @@ static const int bfq_async_charge_factor = 10; /* Default timeout values, in jiffies, approximating CFQ defaults. */ static const int bfq_timeout = (HZ / 8); -struct kmem_cache *bfq_pool; +static struct kmem_cache *bfq_pool; /* Below this threshold (in ns), we consider thinktime immediate. */ #define BFQ_MIN_TT (2 * NSEC_PER_MSEC) @@ -1868,7 +1868,7 @@ static bool bfq_may_be_close_cooperator(struct bfq_queue *bfqq, * positives. In case bfqq is weight-raised, such false positives * would evidently degrade latency guarantees for bfqq. */ -bool wr_from_too_long(struct bfq_queue *bfqq) +static bool wr_from_too_long(struct bfq_queue *bfqq) { return bfqq->wr_coeff > 1 && time_is_before_jiffies(bfqq->last_wr_start_finish + @@ -2298,7 +2298,7 @@ static unsigned long bfq_calc_max_budget(struct bfq_data *bfqd) * function of the estimated peak rate. See comments on * bfq_calc_max_budget(), and on T_slow and T_fast arrays. */ -void update_thr_responsiveness_params(struct bfq_data *bfqd) +static void update_thr_responsiveness_params(struct bfq_data *bfqd) { int dev_type = blk_queue_nonrot(bfqd->queue); @@ -2333,7 +2333,7 @@ void update_thr_responsiveness_params(struct bfq_data *bfqd) BFQ_RATE_SHIFT); } -void bfq_reset_rate_computation(struct bfq_data *bfqd, struct request *rq) +static void bfq_reset_rate_computation(struct bfq_data *bfqd, struct request *rq) { if (rq != NULL) { /* new rq dispatch now, reset accordingly */ bfqd->last_dispatch = bfqd->first_dispatch = ktime_get_ns() ; @@ -2350,7 +2350,7 @@ void bfq_reset_rate_computation(struct bfq_data *bfqd, struct request *rq) bfqd->tot_sectors_dispatched); } -void bfq_update_rate_reset(struct bfq_data *bfqd, struct request *rq) +static void bfq_update_rate_reset(struct bfq_data *bfqd, struct request *rq) { u32 rate, weight, divisor; @@ -2515,7 +2515,7 @@ void bfq_update_rate_reset(struct bfq_data *bfqd, struct request *rq) * of the observed dispatch rate. The function assumes to be invoked * on every request dispatch. */ -void bfq_update_peak_rate(struct bfq_data *bfqd, struct request *rq) +static void bfq_update_peak_rate(struct bfq_data *bfqd, struct request *rq) { u64 now_ns = ktime_get_ns(); From 588042943eab576d65c0ebf57ccf57b82211e7c2 Mon Sep 17 00:00:00 2001 From: Paolo Valente Date: Fri, 3 Feb 2017 11:53:01 +0100 Subject: [PATCH 03/10] BUGFIX: Put async queues on exit also without cgroups BUGFIX: The putting of async queues on scheduler exit was missing in case cgroups support was not active. This fix adds the missing operation. Signed-off-by: Paolo Valente --- block/bfq-cgroup.c | 3 +++ block/bfq-iosched.c | 11 ++++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index bbaecd0..a5f8dc1 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -1140,6 +1140,9 @@ static inline void bfqg_stats_update_idle_time(struct bfq_group *bfqg) { } static inline void bfqg_stats_set_start_idle_time(struct bfq_group *bfqg) { } static inline void bfqg_stats_update_avg_queue_size(struct bfq_group *bfqg) { } +static void bfq_bfqq_move(struct bfq_data *bfqd, struct bfq_queue *bfqq, + struct bfq_group *bfqg) {} + static void bfq_init_entity(struct bfq_entity *entity, struct bfq_group *bfqg) { diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 98a1acd..517f513 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -4093,7 +4093,13 @@ static struct bfq_queue *bfq_get_queue(struct bfq_data *bfqd, * prune it. */ if (async_bfqq) { - bfqq->ref++; + bfqq->ref++; /* + * Extra group reference, w.r.t. sync + * queue. This extra reference is removed + * only if bfqq->bfqg disappears, to + * guarantee that this queue is not freed + * until its group goes away. + */ bfq_log_bfqq(bfqd, bfqq, "get_queue, bfqq not in async: %p, %d", bfqq, bfqq->ref); *async_bfqq = bfqq; @@ -4697,7 +4703,6 @@ static void bfq_shutdown_timer_wq(struct bfq_data *bfqd) cancel_work_sync(&bfqd->unplug_work); } -#ifdef CONFIG_BFQ_GROUP_IOSCHED static void __bfq_put_async_bfqq(struct bfq_data *bfqd, struct bfq_queue **bfqq_ptr) { @@ -4730,7 +4735,6 @@ static void bfq_put_async_queues(struct bfq_data *bfqd, struct bfq_group *bfqg) __bfq_put_async_bfqq(bfqd, &bfqg->async_idle_bfqq); } -#endif static void bfq_exit_queue(struct elevator_queue *e) { @@ -4755,6 +4759,7 @@ static void bfq_exit_queue(struct elevator_queue *e) #ifdef CONFIG_BFQ_GROUP_IOSCHED blkcg_deactivate_policy(q, &blkcg_policy_bfq); #else + bfq_put_async_queues(bfqd, bfqd->root_group); kfree(bfqd->root_group); #endif From 737da1cfab00974c57bdc132f93056e5c5da2cbb Mon Sep 17 00:00:00 2001 From: Paolo Valente Date: Sat, 4 Feb 2017 17:54:35 +0100 Subject: [PATCH 04/10] Fix check of the percentage of sequential dispatches BUGFIX: In the peak-rate estimator, there was a serious error in the check that the percentage of sequential I/O-request dispatches was high enough to trigger an update of the peak-rate estimate. This commit fixes that check. Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 517f513..d603cf9 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -2400,7 +2400,7 @@ static void bfq_update_rate_reset(struct bfq_data *bfqd, struct request *rq) * total, and rate is below the current estimated peak rate * - rate is unreasonably high (> 20M sectors/sec) */ - if ((bfqd->peak_rate_samples > (3 * bfqd->sequential_samples)>>2 && + if ((bfqd->sequential_samples < (3 * bfqd->peak_rate_samples)>>2 && rate <= bfqd->peak_rate) || rate > 20< Date: Sat, 4 Feb 2017 18:03:06 +0100 Subject: [PATCH 05/10] Better tune weight-raising for slow flash-based devices IMPROVEMENT Luca Miccio has run a few responsiveness tests on recent Android systems with average-speed storage devices. These tests have shown that the following BFQ parameter was too low for these systems: reference duration for slow storage devices of weight raising for interactive applications. This commit raises that duration to a value that is yelding optimal results in our tests. Signed-off-by: Luca Miccio Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index d603cf9..ba82d8f 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -5260,7 +5260,7 @@ static int __init bfq_init(void) * be run for a long time. */ T_slow[0] = msecs_to_jiffies(3500); /* actually 4 sec */ - T_slow[1] = msecs_to_jiffies(1000); /* actually 1.5 sec */ + T_slow[1] = msecs_to_jiffies(6000); /* actually 6.5 sec */ T_fast[0] = msecs_to_jiffies(7000); /* actually 8 sec */ T_fast[1] = msecs_to_jiffies(2500); /* actually 3 sec */ From 905e1281d01d0abcb151530cc362ad173b9b2959 Mon Sep 17 00:00:00 2001 From: Paolo Valente Date: Sat, 4 Feb 2017 18:16:59 +0100 Subject: [PATCH 06/10] BFQ-v8r8-rc1 Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 2 +- block/bfq.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index ba82d8f..7ffc167 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -5234,7 +5234,7 @@ static struct blkcg_policy blkcg_policy_bfq = { static int __init bfq_init(void) { int ret; - char msg[60] = "BFQ I/O-scheduler: v8r7"; + char msg[60] = "BFQ I/O-scheduler: v8r8-rc1"; #ifdef CONFIG_BFQ_GROUP_IOSCHED ret = blkcg_policy_register(&blkcg_policy_bfq); diff --git a/block/bfq.h b/block/bfq.h index bef8244..7b12f3c 100644 --- a/block/bfq.h +++ b/block/bfq.h @@ -1,5 +1,5 @@ /* - * BFQ v8r7 for 4.9.0: data structures and common functions prototypes. + * BFQ v8r8-rc1 for 4.10.0: data structures and common functions prototypes. * * Based on ideas and code from CFQ: * Copyright (C) 2003 Jens Axboe From 3955e8c82ce05297fb9e2c5beecbd8df9312abb3 Mon Sep 17 00:00:00 2001 From: Paolo Valente Date: Sun, 5 Feb 2017 18:50:51 +0100 Subject: [PATCH 07/10] Avoid a second dispatch in case of budget exhaustion IMPROVEMENT This commit anticipates the complete check of budget exhaustion, for the in-service bfq_queue, to when the next bfq_queue to serve is selected (during a dispatch operation). This enables a new bfq_queue to be immediately selected for service in case the in-service bfq_queue has actually exhausted its budget. As a consequence, a second dispatch invocation is not needed any more, to have a new request dispatched. To implement this improvement, this commit implements a further improvement too: the field next_rq of a bfq_queue now always contains the actual next request to dispatch (or NULL if the bfq_queue is empty). Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 122 +++++++++++++++++++++++++++------------------------- 1 file changed, 63 insertions(+), 59 deletions(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 7ffc167..2796927 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -519,13 +519,45 @@ static void bfq_weights_tree_remove(struct bfq_data *bfqd, entity->weight_counter = NULL; } +/* + * Return expired entry, or NULL to just start from scratch in rbtree. + */ +static struct request *bfq_check_fifo(struct bfq_queue *bfqq, + struct request *last) +{ + struct request *rq; + + if (bfq_bfqq_fifo_expire(bfqq)) + return NULL; + + bfq_mark_bfqq_fifo_expire(bfqq); + + rq = rq_entry_fifo(bfqq->fifo.next); + + if (rq == last || ktime_get_ns() < rq->fifo_time) + return NULL; + + bfq_log_bfqq(bfqq->bfqd, bfqq, "check_fifo: returned %p", rq); + BUG_ON(RB_EMPTY_NODE(&rq->rb_node)); + return rq; +} + static struct request *bfq_find_next_rq(struct bfq_data *bfqd, struct bfq_queue *bfqq, struct request *last) { struct rb_node *rbnext = rb_next(&last->rb_node); struct rb_node *rbprev = rb_prev(&last->rb_node); - struct request *next = NULL, *prev = NULL; + struct request *next, *prev = NULL; + + BUG_ON(list_empty(&bfqq->fifo)); + + /* Follow expired path, else get first next available. */ + next = bfq_check_fifo(bfqq, last); + if (next) { + BUG_ON(next == last); + return next; + } BUG_ON(RB_EMPTY_NODE(&last->rb_node)); @@ -1523,11 +1555,12 @@ static void bfq_remove_request(struct request *rq) elv_rb_del(&bfqq->sort_list, rq); if (RB_EMPTY_ROOT(&bfqq->sort_list)) { + bfqq->next_rq = NULL; + BUG_ON(bfqq->entity.budget < 0); if (bfq_bfqq_busy(bfqq) && bfqq != bfqd->in_service_queue) { bfq_del_bfqq_busy(bfqd, bfqq, false); - /* bfqq emptied. In normal operation, when * bfqq is empty, bfqq->entity.service and * bfqq->entity.budget must contain, @@ -2616,29 +2649,6 @@ static void bfq_dispatch_insert(struct request_queue *q, struct request *rq) elv_dispatch_sort(q, rq); } -/* - * Return expired entry, or NULL to just start from scratch in rbtree. - */ -static struct request *bfq_check_fifo(struct bfq_queue *bfqq) -{ - struct request *rq = NULL; - - if (bfq_bfqq_fifo_expire(bfqq)) - return NULL; - - bfq_mark_bfqq_fifo_expire(bfqq); - - if (list_empty(&bfqq->fifo)) - return NULL; - - rq = rq_entry_fifo(bfqq->fifo.next); - - if (ktime_get_ns() < rq->fifo_time) - return NULL; - - return rq; -} - static void __bfq_bfqq_expire(struct bfq_data *bfqd, struct bfq_queue *bfqq) { BUG_ON(bfqq != bfqd->in_service_queue); @@ -3504,14 +3514,29 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd) !bfq_bfqq_must_idle(bfqq)) goto expire; +check_queue: + /* + * This loop is rarely executed more than once. Even when it + * happens, it is much more convenient to re-execute this loop + * than to return NULL and trigger a new dispatch to get a + * request served. + */ next_rq = bfqq->next_rq; /* * If bfqq has requests queued and it has enough budget left to * serve them, keep the queue, otherwise expire it. */ if (next_rq) { + BUG_ON(RB_EMPTY_ROOT(&bfqq->sort_list)); + if (bfq_serv_to_charge(next_rq, bfqq) > bfq_bfqq_budget_left(bfqq)) { + /* + * Expire the queue for budget exhaustion, + * which makes sure that the next budget is + * enough to serve the next request, even if + * it comes from the fifo expired path. + */ reason = BFQ_BFQQ_BUDGET_EXHAUSTED; goto expire; } else { @@ -3559,9 +3584,16 @@ static struct bfq_queue *bfq_select_queue(struct bfq_data *bfqd) bfq_bfqq_expire(bfqd, bfqq, false, reason); new_queue: bfqq = bfq_set_in_service_queue(bfqd); - bfq_log(bfqd, "select_queue: new queue %d returned", - bfqq ? bfqq->pid : 0); + if (bfqq) { + bfq_log_bfqq(bfqd, bfqq, "select_queue: checking new queue"); + goto check_queue; + } keep_queue: + if (bfqq) + bfq_log_bfqq(bfqd, bfqq, "select_queue: returned this queue"); + else + bfq_log(bfqd, "select_queue: no queue returned"); + return bfqq; } @@ -3627,45 +3659,17 @@ static int bfq_dispatch_request(struct bfq_data *bfqd, struct bfq_queue *bfqq) { int dispatched = 0; - struct request *rq; + struct request *rq = bfqq->next_rq; unsigned long service_to_charge; BUG_ON(RB_EMPTY_ROOT(&bfqq->sort_list)); - - /* Follow expired path, else get first next available. */ - rq = bfq_check_fifo(bfqq); - if (!rq) - rq = bfqq->next_rq; + BUG_ON(!rq); service_to_charge = bfq_serv_to_charge(rq, bfqq); - if (service_to_charge > bfq_bfqq_budget_left(bfqq)) { - /* - * This may happen if the next rq is chosen in fifo order - * instead of sector order. The budget is properly - * dimensioned to be always sufficient to serve the next - * request only if it is chosen in sector order. The reason - * is that it would be quite inefficient and little useful - * to always make sure that the budget is large enough to - * serve even the possible next rq in fifo order. - * In fact, requests are seldom served in fifo order. - * - * Expire the queue for budget exhaustion, and make sure - * that the next act_budget is enough to serve the next - * request, even if it comes from the fifo expired path. - */ - bfqq->next_rq = rq; - /* - * Since this dispatch is failed, make sure that - * a new one will be performed - */ - if (!bfqd->rq_in_driver) - bfq_schedule_dispatch(bfqd); - BUG_ON(bfqq->entity.budget < bfqq->entity.service); - goto expire; - } + BUG_ON(service_to_charge > bfq_bfqq_budget_left(bfqq)); BUG_ON(bfqq->entity.budget < bfqq->entity.service); - /* Finally, insert request into driver dispatch list. */ + bfq_bfqq_served(bfqq, service_to_charge); BUG_ON(bfqq->entity.budget < bfqq->entity.service); From 99a658b6797c828d52466c5ceef93cf9b3bd1268 Mon Sep 17 00:00:00 2001 From: Paolo Valente Date: Sun, 5 Feb 2017 22:56:25 +0100 Subject: [PATCH 08/10] BFQ-v8r8-rc2 Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 4 ++-- block/bfq.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 2796927..5bfeb16 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -9,7 +9,7 @@ * * Copyright (C) 2015 Paolo Valente * - * Copyright (C) 2016 Paolo Valente + * Copyright (C) 2017 Paolo Valente * * Licensed under the GPL-2 as detailed in the accompanying COPYING.BFQ * file. @@ -5238,7 +5238,7 @@ static struct blkcg_policy blkcg_policy_bfq = { static int __init bfq_init(void) { int ret; - char msg[60] = "BFQ I/O-scheduler: v8r8-rc1"; + char msg[60] = "BFQ I/O-scheduler: v8r8-rc2"; #ifdef CONFIG_BFQ_GROUP_IOSCHED ret = blkcg_policy_register(&blkcg_policy_bfq); diff --git a/block/bfq.h b/block/bfq.h index 7b12f3c..a08e8a6 100644 --- a/block/bfq.h +++ b/block/bfq.h @@ -1,5 +1,5 @@ /* - * BFQ v8r8-rc1 for 4.10.0: data structures and common functions prototypes. + * BFQ v8r8-rc2 for 4.10.0: data structures and common functions prototypes. * * Based on ideas and code from CFQ: * Copyright (C) 2003 Jens Axboe @@ -9,7 +9,7 @@ * * Copyright (C) 2015 Paolo Valente * - * Copyright (C) 2016 Paolo Valente + * Copyright (C) 2017 Paolo Valente */ #ifndef _BFQ_H From 0796dc37acb561ef9c544a68045ff9a016b1ccab Mon Sep 17 00:00:00 2001 From: Oleksandr Natalenko Date: Mon, 20 Feb 2017 16:02:07 +0100 Subject: [PATCH 09/10] block/bfq-cgroup: fix bfq_bic_update_cgroup() API bfq_bic_update_cgroup() should return nothing even if CONFIG_BFQ_GROUP_IOSCHED is disabled. Signed-off-by: Oleksandr Natalenko --- block/bfq-cgroup.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index a5f8dc1..de045cf 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -1157,13 +1157,7 @@ static void bfq_init_entity(struct bfq_entity *entity, entity->sched_data = &bfqg->sched_data; } -static struct bfq_group * -bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio) -{ - struct bfq_data *bfqd = bic_to_bfqd(bic); - - return bfqd->root_group; -} +static void bfq_bic_update_cgroup(struct bfq_io_cq *bic, struct bio *bio) {} static void bfq_end_wr_async(struct bfq_data *bfqd) { From 99b8292bad900ffa16aea9cc9df7a68954026829 Mon Sep 17 00:00:00 2001 From: Paolo Valente Date: Thu, 23 Feb 2017 12:28:00 +0100 Subject: [PATCH 10/10] BFQ-v8r8 Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 2 +- block/bfq.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 5bfeb16..6348d55 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -5238,7 +5238,7 @@ static struct blkcg_policy blkcg_policy_bfq = { static int __init bfq_init(void) { int ret; - char msg[60] = "BFQ I/O-scheduler: v8r8-rc2"; + char msg[60] = "BFQ I/O-scheduler: v8r8"; #ifdef CONFIG_BFQ_GROUP_IOSCHED ret = blkcg_policy_register(&blkcg_policy_bfq); diff --git a/block/bfq.h b/block/bfq.h index a08e8a6..2a2bc30 100644 --- a/block/bfq.h +++ b/block/bfq.h @@ -1,5 +1,5 @@ /* - * BFQ v8r8-rc2 for 4.10.0: data structures and common functions prototypes. + * BFQ v8r8 for 4.10.0: data structures and common functions prototypes. * * Based on ideas and code from CFQ: * Copyright (C) 2003 Jens Axboe 0