From patchwork Tue Jan 29 11:06:30 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Valente X-Patchwork-Id: 156968 Delivered-To: patch@linaro.org Received: by 2002:a02:48:0:0:0:0:0 with SMTP id 69csp4523371jaa; Tue, 29 Jan 2019 03:07:55 -0800 (PST) X-Google-Smtp-Source: ALg8bN7YIPctNl/cO92oJIzk0XO1GVUhdFR9qT/dN7Fk8AsgQX8fVxF5WnL3jogr9d2XOkHNaRQn X-Received: by 2002:a63:5b48:: with SMTP id l8mr23379488pgm.80.1548760075192; Tue, 29 Jan 2019 03:07:55 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1548760075; cv=none; d=google.com; s=arc-20160816; b=BOY1ewjBiMR1MnbOhWBF2TdIuSI4Y5VoJROb9+AmhHQXO9qRLhqWLJ+BobymCY2LED 0qNQ3/EE6kyINH1WDp2uLguhApIR8tXMwObKG9HYthVgB/jZk4vo6jLcKuBQYWc5YekJ p326uhenE50EHIUpfLAICAozAR7BJjJmdG3W3Fhv0GjjRv3YcLNubxWc4awhBoYqicWI rywtu420B6lUF9wObE8iqluEtPrSCPGTsNg5920TRwCQ1f3JcVCATFNom+kjjg/uk9xJ E0werySw9ZG8dqZOSBEmmH4immeooN3Yj/fYlKSstwVC8ncDKpMvW5RsnhgdqHvfSOds mK8A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=QUU3CUC8cc8cju82LpP1ETMRx5xYgBZFuzHMif5PKgE=; b=PxJ6UZG8YDFDtCoVsoZccK8yIzlMEtmblZuCMcUhOaFUIKdco/9+UyMe3iXVe4y3fG BtT8YHc/FdyI3YokMUOWsLC1unge5rqoIKM66YJvuYVAZWlDrkkJn8kX57oqwmzeAZRE ic4rghuh5GZPRq47l4Q1Sp8Mfc30MiQFb/zx2sg9FhTe/GkDDV6831/sss9VUx4fLOS8 KV1KHjyI9PsXBbTKl1Jyyfr4cOibYeDbf3p/vPd+h6gnLUXGhF5a9jYjlqCzKUBRbK9v QbklnujQF9YDuq8je2CxpQhMLBAV8cZMef1dfxcXxO5WtjTjOd/xrgmstkHsjL2MOvcZ 1O+A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=I7GipiBy; 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 b5si14103712pfg.121.2019.01.29.03.07.54; Tue, 29 Jan 2019 03:07:55 -0800 (PST) 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.s=google header.b=I7GipiBy; 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 S1728686AbfA2LHw (ORCPT + 31 others); Tue, 29 Jan 2019 06:07:52 -0500 Received: from mail-wm1-f67.google.com ([209.85.128.67]:52742 "EHLO mail-wm1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726893AbfA2LHI (ORCPT ); Tue, 29 Jan 2019 06:07:08 -0500 Received: by mail-wm1-f67.google.com with SMTP id m1so17344315wml.2 for ; Tue, 29 Jan 2019 03:07:05 -0800 (PST) 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 :mime-version:content-transfer-encoding; bh=QUU3CUC8cc8cju82LpP1ETMRx5xYgBZFuzHMif5PKgE=; b=I7GipiBy1jwfZzcBnanIeEjUnph/c3fRUsd0smPewaxwZgXl13jxCG7lJDGG7pRnWe YnObOi6zGYuBEL6aqupczOMweeg9M3PBxM3SgnsWgpKWDIyhQRz6U+vCKr0Ur6gjFbxH jvRqkwyKPT6zMzU2EqfnFgpdeCH0endlgmAd4= 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:mime-version:content-transfer-encoding; bh=QUU3CUC8cc8cju82LpP1ETMRx5xYgBZFuzHMif5PKgE=; b=kXEXWYB4BeDfD4x4c3qoNcYuEwESkD1GaDBrFIie/dA7CKMWTZAvGndWwGsJpvbqF+ X+0riEpTYHL0NMR5qaHUbA+IKbxeZArgkB0/VWH1xbP5WubYJILcerbxO+DwhlzXRWvI 9yYudPaDDxRHy3o9qYADRGURk7HP2U++Sj6O57eegrL10qVz0a6NNe6fh8v/BtX+eYWl +vQT/Mibi6BX5iIVjpgMSZJ892xvMUdNisNMnENYm7j/z0gZudyh+fK8tZWOwWF6kFxz n6gNtqO0yr3Q4SVuDd8VTDU8QW7MShCcr2JhCX6mE2NYLLHqEh8FcO83hUUVkAigDeWB Q51A== X-Gm-Message-State: AJcUukeoozIfUM6l82tGSROWuCxcfgXWMdS2phepiPS0oq7yFcWERbq1 96RSQi53MEGmijTP5lkKuNCBEg== X-Received: by 2002:a1c:2457:: with SMTP id k84mr20732866wmk.139.1548760024837; Tue, 29 Jan 2019 03:07:04 -0800 (PST) Received: from localhost.localdomain ([88.147.67.218]) by smtp.gmail.com with ESMTPSA id s132sm2066112wmf.28.2019.01.29.03.07.03 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 29 Jan 2019 03:07:04 -0800 (PST) From: Paolo Valente To: Jens Axboe Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, ulf.hansson@linaro.org, linus.walleij@linaro.org, broonie@kernel.org, bfq-iosched@googlegroups.com, oleksandr@natalenko.name, mancha@tower-research.com, Paolo Valente Subject: [PATCH BUGFIX IMPROVEMENT 06/14] block, bfq: split function bfq_better_to_idle Date: Tue, 29 Jan 2019 12:06:30 +0100 Message-Id: <20190129110638.12652-7-paolo.valente@linaro.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190129110638.12652-1-paolo.valente@linaro.org> References: <20190129110638.12652-1-paolo.valente@linaro.org> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is a preparatory commit for commits that need to check only one of the two main reasons for idling. This change should also improve the quality of the code a little bit, by splitting a function that contains very long, non-trivial and little related comments. Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 155 +++++++++++++++++++++++--------------------- 1 file changed, 82 insertions(+), 73 deletions(-) -- 2.20.1 diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 6bfbfa65610b..2756f4b1432b 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -3404,53 +3404,13 @@ static bool bfq_may_expire_for_budg_timeout(struct bfq_queue *bfqq) bfq_bfqq_budget_timeout(bfqq); } -/* - * For a queue that becomes empty, device idling is allowed only if - * this function returns true for the queue. As a consequence, since - * device idling plays a critical role in both throughput boosting and - * service guarantees, the return value of this function plays a - * critical role in both these aspects as well. - * - * In a nutshell, this function returns true only if idling is - * beneficial for throughput or, even if detrimental for throughput, - * idling is however necessary to preserve service guarantees (low - * latency, desired throughput distribution, ...). In particular, on - * NCQ-capable devices, this function tries to return false, so as to - * help keep the drives' internal queues full, whenever this helps the - * device boost the throughput without causing any service-guarantee - * issue. - * - * In more detail, the return value of this function is obtained by, - * first, computing a number of boolean variables that take into - * account throughput and service-guarantee issues, and, then, - * combining these variables in a logical expression. Most of the - * issues taken into account are not trivial. We discuss these issues - * individually while introducing the variables. - */ -static bool bfq_better_to_idle(struct bfq_queue *bfqq) +static bool idling_boosts_thr_without_issues(struct bfq_data *bfqd, + struct bfq_queue *bfqq) { - struct bfq_data *bfqd = bfqq->bfqd; bool rot_without_queueing = !blk_queue_nonrot(bfqd->queue) && !bfqd->hw_tag, bfqq_sequential_and_IO_bound, - idling_boosts_thr, idling_boosts_thr_without_issues, - idling_needed_for_service_guarantees, - asymmetric_scenario; - - if (bfqd->strict_guarantees) - 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; + idling_boosts_thr; bfqq_sequential_and_IO_bound = !BFQQ_SEEKY(bfqq) && bfq_bfqq_IO_bound(bfqq) && bfq_bfqq_has_short_ttime(bfqq); @@ -3482,8 +3442,7 @@ static bool bfq_better_to_idle(struct bfq_queue *bfqq) bfqq_sequential_and_IO_bound); /* - * The value of the next variable, - * idling_boosts_thr_without_issues, is equal to that of + * The return value of this function is equal to that of * idling_boosts_thr, unless a special case holds. In this * special case, described below, idling may cause problems to * weight-raised queues. @@ -3500,32 +3459,35 @@ static bool bfq_better_to_idle(struct bfq_queue *bfqq) * which enqueue several requests in advance, and further * reorder internally-queued requests. * - * For this reason, we force to false the value of - * idling_boosts_thr_without_issues if there are weight-raised - * busy queues. In this case, and if bfqq is not weight-raised, - * this guarantees that the device is not idled for bfqq (if, - * instead, bfqq is weight-raised, then idling will be - * guaranteed by another variable, see below). Combined with - * the timestamping rules of BFQ (see [1] for details), this - * behavior causes bfqq, and hence any sync non-weight-raised - * queue, to get a lower number of requests served, and thus - * to ask for a lower number of requests from the request - * pool, before the busy weight-raised queues get served - * again. This often mitigates starvation problems in the - * presence of heavy write workloads and NCQ, thereby - * guaranteeing a higher application and system responsiveness - * in these hostile scenarios. + * For this reason, we force to false the return value if + * there are weight-raised busy queues. In this case, and if + * bfqq is not weight-raised, this guarantees that the device + * is not idled for bfqq (if, instead, bfqq is weight-raised, + * then idling will be guaranteed by another variable, see + * below). Combined with the timestamping rules of BFQ (see + * [1] for details), this behavior causes bfqq, and hence any + * sync non-weight-raised queue, to get a lower number of + * requests served, and thus to ask for a lower number of + * requests from the request pool, before the busy + * weight-raised queues get served again. This often mitigates + * starvation problems in the presence of heavy write + * workloads and NCQ, thereby guaranteeing a higher + * application and system responsiveness in these hostile + * scenarios. */ - idling_boosts_thr_without_issues = idling_boosts_thr && + return idling_boosts_thr && bfqd->wr_busy_queues == 0; +} +static bool idling_needed_for_service_guarantees(struct bfq_data *bfqd, + struct bfq_queue *bfqq) +{ /* - * There is then a case where idling must be performed not - * for throughput concerns, but to preserve service - * guarantees. + * There is a case where idling must be performed not for + * throughput concerns, but to preserve service guarantees. * * To introduce this case, we can note that allowing the drive - * to enqueue more than one request at a time, and hence + * to enqueue more than one request at a time, and thereby * delegating de facto final scheduling decisions to the * drive's internal scheduler, entails loss of control on the * actual request service order. In particular, the critical @@ -3682,9 +3644,9 @@ static bool bfq_better_to_idle(struct bfq_queue *bfqq) * to let requests be served in the desired order until all * the requests already queued in the device have been served. */ - asymmetric_scenario = (bfqq->wr_coeff > 1 && - bfqd->wr_busy_queues < - bfq_tot_busy_queues(bfqd)) || + bool asymmetric_scenario = (bfqq->wr_coeff > 1 && + bfqd->wr_busy_queues < + bfq_tot_busy_queues(bfqd)) || !bfq_symmetric_scenario(bfqd); /* @@ -3701,17 +3663,64 @@ static bool bfq_better_to_idle(struct bfq_queue *bfqq) * now establish when idling is actually needed to preserve * service guarantees. */ - idling_needed_for_service_guarantees = - asymmetric_scenario && !bfq_bfqq_in_large_burst(bfqq); + return asymmetric_scenario && !bfq_bfqq_in_large_burst(bfqq); +} + +/* + * For a queue that becomes empty, device idling is allowed only if + * this function returns true for that queue. As a consequence, since + * device idling plays a critical role for both throughput boosting + * and service guarantees, the return value of this function plays a + * critical role as well. + * + * In a nutshell, this function returns true only if idling is + * beneficial for throughput or, even if detrimental for throughput, + * idling is however necessary to preserve service guarantees (low + * latency, desired throughput distribution, ...). In particular, on + * NCQ-capable devices, this function tries to return false, so as to + * help keep the drives' internal queues full, whenever this helps the + * device boost the throughput without causing any service-guarantee + * issue. + * + * Most of the issues taken into account to get the return value of + * this function are not trivial. We discuss these issues in the two + * functions providing the main pieces of information needed by this + * function. + */ +static bool bfq_better_to_idle(struct bfq_queue *bfqq) +{ + struct bfq_data *bfqd = bfqq->bfqd; + bool idling_boosts_thr_with_no_issue, idling_needed_for_service_guar; + + if (unlikely(bfqd->strict_guarantees)) + 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; + + idling_boosts_thr_with_no_issue = + idling_boosts_thr_without_issues(bfqd, bfqq); + + idling_needed_for_service_guar = + idling_needed_for_service_guarantees(bfqd, bfqq); /* - * We have now all the components we need to compute the + * We have now the two 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 idling_boosts_thr_without_issues || - idling_needed_for_service_guarantees; + return idling_boosts_thr_with_no_issue || + idling_needed_for_service_guar; } /*