From patchwork Fri Aug 4 05:35:10 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Valente X-Patchwork-Id: 109372 Delivered-To: patch@linaro.org Received: by 10.140.101.6 with SMTP id t6csp1710745qge; Thu, 3 Aug 2017 22:35:53 -0700 (PDT) X-Received: by 10.98.149.132 with SMTP id c4mr1244757pfk.134.1501824953530; Thu, 03 Aug 2017 22:35:53 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1501824953; cv=none; d=google.com; s=arc-20160816; b=ldikYu8OOyqOll1bxF8LiNRWY/+X77WSufxMGD9NtZHelfiL4E0qZxuqAPPINOM8ce 8P846PIKEcBcfqTYaosqEBAQawzavUajs94iCqo0XppF88zs6RE8nh1aOS/y1ZHCJ9Yz TSIfGq5z/76RmwOg72QNuOxey/XU7mv26WegYfYZlPhSqjSmMYeXBU8hHAiac9IhkawE emyQylH4NMFyar2HFQSMn5j5G8oHoIfm5+goVmQtK5LBppgEsMV+oACjUyKQuD22yXoZ XXhzlXTVVxbbnaJ4E2aLBxuGTdl1+Xtd/eqDBaQwoSK2apFChJegzvsnPcUK/I9S6QEd rHog== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:references:in-reply-to:message-id:date :subject:cc:to:from:dkim-signature:arc-authentication-results; bh=LiVYFmhsorVb6ms4WkDd6pX8CpV+esMEaR9qDkdEO6Y=; b=BcWaS4cEMx1v5T3IGtPfXY8YWbXQY0boSWY9NtZDeQz/0Q94HNYi6UXcsSUA6PC6XA YUsAy5QzPQMYqsSaZz3/CEOiB6H7e/WRz0NcC9Ou78QHhkpm+etAwA2lq9Qv4ShoslQI BzpT3mUcnig87fnloMV6cm5mGk4pJ6/4fno8cToIuXG2nhJQUqj9WOQwJ8Od/1hdxzLM /DApzsxhYKxCQFjX1pilsHfHJ79K5NgHFqBabas1bkDW1tsHPB4eo7rTZkLC+tALRbTk Amjo696GbYhwsBvXnH2KPSuCsQ/9hNo+IyONT6+gZZqpLFfmviicNnH0/qpkrp014XdJ tm/g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.b=h6vf+KTo; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id x190si469187pgd.606.2017.08.03.22.35.53; Thu, 03 Aug 2017 22:35:53 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.b=h6vf+KTo; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751940AbdHDFfu (ORCPT + 25 others); Fri, 4 Aug 2017 01:35:50 -0400 Received: from mail-wm0-f43.google.com ([74.125.82.43]:36416 "EHLO mail-wm0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751283AbdHDFf2 (ORCPT ); Fri, 4 Aug 2017 01:35:28 -0400 Received: by mail-wm0-f43.google.com with SMTP id t201so10913896wmt.1 for ; Thu, 03 Aug 2017 22:35:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=LiVYFmhsorVb6ms4WkDd6pX8CpV+esMEaR9qDkdEO6Y=; b=h6vf+KTo63SQlYUGlGSg7somfe3prY/0dhAJ3ZWHQz+DvmNCJyW6xHk0kmMdltddAj B7g6QiWk5Z8yMvXzopL/CrqyU0wplCrEe7perXirjAZHWZ0nKWI/IHNoifUEgTa+iTKg QdI+UvnL2pM/WTqha+UT03O/41BSMLrmxr6Us= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=LiVYFmhsorVb6ms4WkDd6pX8CpV+esMEaR9qDkdEO6Y=; b=AUyYXN7Hd+sSwpU+3K3fETONA0nz1H7H2XXLvn5I8D5XPtDcomydnjD19Mppp6JXfN YLcxtjICjTVEz7No/slYcOvwT0YY5BzZJ7nyUX+UYWLm1/6JieuNImqdXH7Tf6bzDvrJ vjG0bH/2gHwravDR//Sq3e5sL4STN92SBPqTEMUnjBXiUMK286MzdoS4+QjC6ZAtx6vd xGfdKrMmB0bXmUnHWbf5a8Vt6f7r8DMPkVTux9z5z8ans9We8ya0N/lwbJCK3eFHyEOv ZsxUsDmgCmtcjaRpW4saSQOj/ThpQU+zU5LE0og0WCha907gGvZHA3xlQQ5INUkY6MvS YTJg== X-Gm-Message-State: AHYfb5jqZ5MxVDlEq50v+ZOn1Sb3En3Zqn4TO9DqJbxS1exE8DhZLuHX azZgwFFy5vMZyA4fnLc9Hg== X-Received: by 10.28.226.136 with SMTP id z130mr499850wmg.153.1501824926897; Thu, 03 Aug 2017 22:35:26 -0700 (PDT) Received: from localhost.localdomain ([185.14.11.62]) by smtp.gmail.com with ESMTPSA id 196sm3082142wmg.36.2017.08.03.22.35.25 (version=TLS1 cipher=AES128-SHA bits=128/128); Thu, 03 Aug 2017 22:35:26 -0700 (PDT) From: Paolo Valente To: Jens Axboe Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, ulf.hansson@linaro.org, broonie@kernel.org, lucmiccio@gmail.com, Paolo Valente Subject: [PATCH BUGFIX/IMPROVEMENT V2 1/2] block, bfq: refactor device-idling logic Date: Fri, 4 Aug 2017 07:35:10 +0200 Message-Id: <20170804053511.2389-2-paolo.valente@linaro.org> X-Mailer: git-send-email 2.10.0 In-Reply-To: <20170804053511.2389-1-paolo.valente@linaro.org> References: <20170804053511.2389-1-paolo.valente@linaro.org> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The logic that decides whether to idle the device is scattered across three functions. Almost all of the logic is in the function bfq_bfqq_may_idle, but (1) part of the decision is made in bfq_update_idle_window, and (2) the function bfq_bfqq_must_idle may switch off idling regardless of the output of bfq_bfqq_may_idle. In addition, both bfq_update_idle_window and bfq_bfqq_must_idle make their decisions as a function of parameters that are used, for similar purposes, also in bfq_bfqq_may_idle. This commit addresses these issues by moving all the logic into bfq_bfqq_may_idle. Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 117 +++++++++++++++++++++++++++------------------------- block/bfq-iosched.h | 12 +++--- 2 files changed, 67 insertions(+), 62 deletions(-) -- 2.10.0 diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 436b6ca..ccdc9e6 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -128,7 +128,7 @@ BFQ_BFQQ_FNS(busy); BFQ_BFQQ_FNS(wait_request); BFQ_BFQQ_FNS(non_blocking_wait_rq); BFQ_BFQQ_FNS(fifo_expire); -BFQ_BFQQ_FNS(idle_window); +BFQ_BFQQ_FNS(has_short_ttime); BFQ_BFQQ_FNS(sync); BFQ_BFQQ_FNS(IO_bound); BFQ_BFQQ_FNS(in_large_burst); @@ -731,10 +731,10 @@ bfq_bfqq_resume_state(struct bfq_queue *bfqq, struct bfq_data *bfqd, unsigned int old_wr_coeff = bfqq->wr_coeff; bool busy = bfq_already_existing && bfq_bfqq_busy(bfqq); - if (bic->saved_idle_window) - bfq_mark_bfqq_idle_window(bfqq); + if (bic->saved_has_short_ttime) + bfq_mark_bfqq_has_short_ttime(bfqq); else - bfq_clear_bfqq_idle_window(bfqq); + bfq_clear_bfqq_has_short_ttime(bfqq); if (bic->saved_IO_bound) bfq_mark_bfqq_IO_bound(bfqq); @@ -2012,7 +2012,7 @@ static void bfq_bfqq_save_state(struct bfq_queue *bfqq) return; bic->saved_ttime = bfqq->ttime; - bic->saved_idle_window = bfq_bfqq_idle_window(bfqq); + bic->saved_has_short_ttime = bfq_bfqq_has_short_ttime(bfqq); bic->saved_IO_bound = bfq_bfqq_IO_bound(bfqq); bic->saved_in_large_burst = bfq_bfqq_in_large_burst(bfqq); bic->was_in_burst_list = !hlist_unhashed(&bfqq->burst_list_node); @@ -3038,8 +3038,8 @@ void bfq_bfqq_expire(struct bfq_data *bfqd, } bfq_log_bfqq(bfqd, bfqq, - "expire (%d, slow %d, num_disp %d, idle_win %d)", reason, - slow, bfqq->dispatched, bfq_bfqq_idle_window(bfqq)); + "expire (%d, slow %d, num_disp %d, short_ttime %d)", reason, + slow, bfqq->dispatched, bfq_bfqq_has_short_ttime(bfqq)); /* * Increase, decrease or leave budget unchanged according to @@ -3122,6 +3122,18 @@ static bool bfq_bfqq_may_idle(struct bfq_queue *bfqq) return true; /* + * Idling is performed only if slice_idle > 0. In addition, we + * do not idle if + * (a) bfqq is async + * (b) bfqq is in the idle io prio class: in this case we do + * not idle because we want to minimize the bandwidth that + * queues in this class can steal to higher-priority queues + */ + if (bfqd->bfq_slice_idle == 0 || !bfq_bfqq_sync(bfqq) || + bfq_class_idle(bfqq)) + return false; + + /* * The next variable takes into account the cases where idling * boosts the throughput. * @@ -3142,7 +3154,7 @@ static bool bfq_bfqq_may_idle(struct bfq_queue *bfqq) */ idling_boosts_thr = !bfqd->hw_tag || (!blk_queue_nonrot(bfqd->queue) && bfq_bfqq_IO_bound(bfqq) && - bfq_bfqq_idle_window(bfqq)); + bfq_bfqq_has_short_ttime(bfqq)); /* * The value of the next variable, @@ -3313,16 +3325,13 @@ static bool bfq_bfqq_may_idle(struct bfq_queue *bfqq) asymmetric_scenario && !bfq_bfqq_in_large_burst(bfqq); /* - * We have now all the components we need to compute the return - * value of the function, which is true only if both the following - * conditions hold: - * 1) bfqq is sync, because idling make sense only for sync queues; - * 2) idling either boosts the throughput (without issues), or - * is necessary to preserve service guarantees. + * We have now all the components we need to compute the + * return value of the function, which is true only if idling + * either boosts the throughput (without issues), or is + * necessary to preserve service guarantees. */ - return bfq_bfqq_sync(bfqq) && - (idling_boosts_thr_without_issues || - idling_needed_for_service_guarantees); + return idling_boosts_thr_without_issues || + idling_needed_for_service_guarantees; } /* @@ -3338,10 +3347,7 @@ static bool bfq_bfqq_may_idle(struct bfq_queue *bfqq) */ static bool bfq_bfqq_must_idle(struct bfq_queue *bfqq) { - struct bfq_data *bfqd = bfqq->bfqd; - - return RB_EMPTY_ROOT(&bfqq->sort_list) && bfqd->bfq_slice_idle != 0 && - bfq_bfqq_may_idle(bfqq); + return RB_EMPTY_ROOT(&bfqq->sort_list) && bfq_bfqq_may_idle(bfqq); } /* @@ -3783,7 +3789,6 @@ bfq_set_next_ioprio_data(struct bfq_queue *bfqq, struct bfq_io_cq *bic) case IOPRIO_CLASS_IDLE: bfqq->new_ioprio_class = IOPRIO_CLASS_IDLE; bfqq->new_ioprio = 7; - bfq_clear_bfqq_idle_window(bfqq); break; } @@ -3843,8 +3848,14 @@ static void bfq_init_bfqq(struct bfq_data *bfqd, struct bfq_queue *bfqq, bfq_set_next_ioprio_data(bfqq, bic); if (is_sync) { + /* + * No need to mark as has_short_ttime if in + * idle_class, because no device idling is performed + * for queues in idle class + */ if (!bfq_class_idle(bfqq)) - bfq_mark_bfqq_idle_window(bfqq); + /* tentatively mark as has_short_ttime */ + bfq_mark_bfqq_has_short_ttime(bfqq); bfq_mark_bfqq_sync(bfqq); bfq_mark_bfqq_just_created(bfqq); } else @@ -3985,18 +3996,19 @@ bfq_update_io_seektime(struct bfq_data *bfqd, struct bfq_queue *bfqq, blk_rq_sectors(rq) < BFQQ_SECT_THR_NONROT); } -/* - * Disable idle window if the process thinks too long or seeks so much that - * it doesn't matter. - */ -static void bfq_update_idle_window(struct bfq_data *bfqd, - struct bfq_queue *bfqq, - struct bfq_io_cq *bic) +static void bfq_update_has_short_ttime(struct bfq_data *bfqd, + struct bfq_queue *bfqq, + struct bfq_io_cq *bic) { - int enable_idle; + bool has_short_ttime = true; - /* Don't idle for async or idle io prio class. */ - if (!bfq_bfqq_sync(bfqq) || bfq_class_idle(bfqq)) + /* + * No need to update has_short_ttime if bfqq is async or in + * idle io prio class, or if bfq_slice_idle is zero, because + * no device idling is performed for bfqq in this case. + */ + if (!bfq_bfqq_sync(bfqq) || bfq_class_idle(bfqq) || + bfqd->bfq_slice_idle == 0) return; /* Idle window just restored, statistics are meaningless. */ @@ -4004,27 +4016,22 @@ static void bfq_update_idle_window(struct bfq_data *bfqd, bfqd->bfq_wr_min_idle_time)) return; - enable_idle = bfq_bfqq_idle_window(bfqq); - + /* Think time is infinite if no process is linked to + * bfqq. Otherwise check average think time to + * decide whether to mark as has_short_ttime + */ if (atomic_read(&bic->icq.ioc->active_ref) == 0 || - bfqd->bfq_slice_idle == 0 || - (bfqd->hw_tag && BFQQ_SEEKY(bfqq) && - bfqq->wr_coeff == 1)) - enable_idle = 0; - else if (bfq_sample_valid(bfqq->ttime.ttime_samples)) { - if (bfqq->ttime.ttime_mean > bfqd->bfq_slice_idle && - bfqq->wr_coeff == 1) - enable_idle = 0; - else - enable_idle = 1; - } - bfq_log_bfqq(bfqd, bfqq, "update_idle_window: enable_idle %d", - enable_idle); + (bfq_sample_valid(bfqq->ttime.ttime_samples) && + bfqq->ttime.ttime_mean > bfqd->bfq_slice_idle)) + has_short_ttime = false; + + bfq_log_bfqq(bfqd, bfqq, "update_has_short_ttime: has_short_ttime %d", + has_short_ttime); - if (enable_idle) - bfq_mark_bfqq_idle_window(bfqq); + if (has_short_ttime) + bfq_mark_bfqq_has_short_ttime(bfqq); else - bfq_clear_bfqq_idle_window(bfqq); + bfq_clear_bfqq_has_short_ttime(bfqq); } /* @@ -4040,14 +4047,12 @@ static void bfq_rq_enqueued(struct bfq_data *bfqd, struct bfq_queue *bfqq, bfqq->meta_pending++; bfq_update_io_thinktime(bfqd, bfqq); + bfq_update_has_short_ttime(bfqd, bfqq, bic); bfq_update_io_seektime(bfqd, bfqq, rq); - if (bfqq->entity.service > bfq_max_budget(bfqd) / 8 || - !BFQQ_SEEKY(bfqq)) - bfq_update_idle_window(bfqd, bfqq, bic); bfq_log_bfqq(bfqd, bfqq, - "rq_enqueued: idle_window=%d (seeky %d)", - bfq_bfqq_idle_window(bfqq), BFQQ_SEEKY(bfqq)); + "rq_enqueued: has_short_ttime=%d (seeky %d)", + bfq_bfqq_has_short_ttime(bfqq), BFQQ_SEEKY(bfqq)); bfqq->last_request_pos = blk_rq_pos(rq) + blk_rq_sectors(rq); diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h index 63e771a..53e2907 100644 --- a/block/bfq-iosched.h +++ b/block/bfq-iosched.h @@ -348,11 +348,11 @@ struct bfq_io_cq { uint64_t blkcg_serial_nr; /* the current blkcg serial */ #endif /* - * Snapshot of the idle window before merging; taken to - * remember this value while the queue is merged, so as to be - * able to restore it in case of split. + * Snapshot of the has_short_time flag before merging; taken + * to remember its value while the queue is merged, so as to + * be able to restore it in case of split. */ - bool saved_idle_window; + bool saved_has_short_ttime; /* * Same purpose as the previous two fields for the I/O bound * classification of a queue. @@ -626,7 +626,7 @@ enum bfqq_state_flags { * without idling the device */ BFQQF_fifo_expire, /* FIFO checked in this slice */ - BFQQF_idle_window, /* slice idling enabled */ + BFQQF_has_short_ttime, /* queue has a short think time */ BFQQF_sync, /* synchronous queue */ BFQQF_IO_bound, /* * bfqq has timed-out at least once @@ -655,7 +655,7 @@ BFQ_BFQQ_FNS(busy); BFQ_BFQQ_FNS(wait_request); BFQ_BFQQ_FNS(non_blocking_wait_rq); BFQ_BFQQ_FNS(fifo_expire); -BFQ_BFQQ_FNS(idle_window); +BFQ_BFQQ_FNS(has_short_ttime); BFQ_BFQQ_FNS(sync); BFQ_BFQQ_FNS(IO_bound); BFQ_BFQQ_FNS(in_large_burst);