From patchwork Mon Jul 3 08:00: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: 106886 Delivered-To: patch@linaro.org Received: by 10.140.101.44 with SMTP id t41csp5485821qge; Mon, 3 Jul 2017 01:02:17 -0700 (PDT) X-Received: by 10.99.246.69 with SMTP id u5mr9133728pgj.173.1499068937846; Mon, 03 Jul 2017 01:02:17 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1499068937; cv=none; d=google.com; s=arc-20160816; b=SFW3KRaH/44BO1vZJ3/5zahhm3OZ9f1VabDKhob20FbfrAzcXzITaQmrCDs63BCfXl xb0JkiJhTBPq/NLNhELL1H+S/k71+wujWLP0AxETHgICSBfFgjZd0H/ZNFU1BGURm87Y cq09EeAzR2pYWlH7ikDdTHvAAOEguNBzqiD119VoKYistWOAHnSIvJxPtImipZ6V6WBe Bz2a/VeLT0zsQpTVq5IOY9jdrDgk0iYf5fMv48yaa/wlXAgd1A7ac0/M0DjFhQ/yalad hnoreT9+HHEk9xwEqmn3wXfh0ywPKOTE74ix0g3Rr78HTkwOcUpllJ1g/xP1sfK48S0q qLFQ== 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=96rvErtb3ECYUYk5PB21F8pFteYaiIchZZuC1Ci2GdU=; b=iT7zPOgMUOY5rbuXZ+h7VV/ncm1fAdL8mQE1sNN5td5s4E+oi+ZVqeFgnYxAVsnAZu JP0U/QjUiMvgdq3zELf6aGUynSt7USSagJhGHs5QfBktTOVSZiJP/twgt8hkKLa+V2td LeERFUYwaGOktUC6dTD2/HsAU0nTfbc+WBDWx04/m0cibT7HmdRWjtu5La6nLDT+XTtz 50PAdw/Z008UPpWchN+6S8fhbdWKjfV2IsN/9ucquMuK4p/a3J/ivZzBN+ozaOtQjSWW Y56Q5D+8s287RePWF0HH9SarRLVnA3IMndJMPrOBy18SfMJj+HCh14XE8reehTGfjf+6 PHgQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.b=cigaOji9; 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 s15si9107252pgr.230.2017.07.03.01.02.17; Mon, 03 Jul 2017 01:02:17 -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=cigaOji9; 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 S1753161AbdGCIBe (ORCPT + 25 others); Mon, 3 Jul 2017 04:01:34 -0400 Received: from mail-wr0-f181.google.com ([209.85.128.181]:34758 "EHLO mail-wr0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753093AbdGCIBD (ORCPT ); Mon, 3 Jul 2017 04:01:03 -0400 Received: by mail-wr0-f181.google.com with SMTP id 77so229951392wrb.1 for ; Mon, 03 Jul 2017 01:01:02 -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=96rvErtb3ECYUYk5PB21F8pFteYaiIchZZuC1Ci2GdU=; b=cigaOji9VL72XYhuQY6MYokvMO/YVtlH/c/fxfSSm8HQBdFMXWxI8Rpnh57evgMdC1 FyQJGG2/iaMwTVaYyFg7nUBEu2qmH2fcXMO6T7KZC/QP0fiJPvNwQtQynxMZm59pJEPl /q99hkCzWIMVz1CovfpYFPdWXNxGbePA82hgM= 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=96rvErtb3ECYUYk5PB21F8pFteYaiIchZZuC1Ci2GdU=; b=m2LGwU/3YWtRTiAzJHy+fjdVy7hm2uW2ApV6MrP/WRcsNgMrRvI/GU0aSxgYKho878 /y9Ko/kv29bNTmb4qPfqlm5EpIbfQrs506gBquIoeZoRQxQl5/C2PZO9SGbkcEwar6jX kTY0/0F6wAjBR7fxpaTnZZLRjYeDaYToIv+T9KugAOXufv7VjsE94nNGILA4mQbF+egT EhcJLrgl1E5xhhzzStb3etbu9YHV8sUp7QNJL7xMOm7eTvOnH9aw7WhlmceWbVXPfiSb e6e4J2cE5SR0TPBGPELDbDlDoJK+4xBvJPZmfpG5dSp+bDwXWO4y4zs17GiWj7wDIQd4 tQCA== X-Gm-Message-State: AKS2vOznULno9wNGXLjJBjSmlabgUoCY0EhdMiFOvuZu4htvhhbGso6G BfjTKV/2V+8ceCX6 X-Received: by 10.223.174.147 with SMTP id y19mr35181557wrc.155.1499068861285; Mon, 03 Jul 2017 01:01:01 -0700 (PDT) Received: from localhost.localdomain ([158.148.93.81]) by smtp.gmail.com with ESMTPSA id r142sm6134517wmg.24.2017.07.03.01.00.59 (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 03 Jul 2017 01:01:00 -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, Paolo Valente Subject: [PATCH BUGFIX] block, bfq: don't change ioprio class for a bfq_queue on a service tree Date: Mon, 3 Jul 2017 10:00:10 +0200 Message-Id: <20170703080010.1709-2-paolo.valente@linaro.org> X-Mailer: git-send-email 2.10.0 In-Reply-To: <20170703080010.1709-1-paolo.valente@linaro.org> References: <20170703080010.1709-1-paolo.valente@linaro.org> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On each deactivation or re-scheduling (after being served) of a bfq_queue, BFQ invokes the function __bfq_entity_update_weight_prio(), to perform pending updates of ioprio, weight and ioprio class for the bfq_queue. BFQ also invokes this function on I/O-request dispatches, to raise or lower weights more quickly when needed, thereby improving latency. However, the entity representing the bfq_queue may be on the active (sub)tree of a service tree when this happens, and, although with a very low probability, the bfq_queue may happen to also have a pending change of its ioprio class. If both conditions hold when __bfq_entity_update_weight_prio() is invoked, then the entity moves to a sort of hybrid state: the new service tree for the entity, as returned by bfq_entity_service_tree(), differs from service tree on which the entity still is. The functions that handle activations and deactivations of entities do not cope with such a hybrid state (and would need to become more complex to cope). This commit addresses this issue by just making __bfq_entity_update_weight_prio() not perform also a possible pending change of ioprio class, when invoked on an I/O-request dispatch for a bfq_queue. Such a change is thus postponed to when __bfq_entity_update_weight_prio() is invoked on deactivation or re-scheduling of the bfq_queue. Reported-by: Marco Piazza Reported-by: Laurentiu Nicola Signed-off-by: Paolo Valente Tested-by: Marco Piazza --- block/bfq-iosched.c | 14 ++++++++++---- block/bfq-iosched.h | 3 ++- block/bfq-wf2q.c | 39 ++++++++++++++++++++++++++++++++++----- 3 files changed, 46 insertions(+), 10 deletions(-) -- 2.10.0 diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index ed93da2..9d4919b 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -3471,11 +3471,17 @@ static void bfq_update_wr_data(struct bfq_data *bfqd, struct bfq_queue *bfqq) } } } - /* Update weight both if it must be raised and if it must be lowered */ + /* + * To improve latency (for this or other queues), immediately + * update weight both if it must be raised and if it must be + * lowered. Since, entity may be on some active tree here, and + * might have a pending change of its ioprio class, invoke + * next function with the last parameter unset (see the + * comments on the function). + */ if ((entity->weight > entity->orig_weight) != (bfqq->wr_coeff > 1)) - __bfq_entity_update_weight_prio( - bfq_entity_service_tree(entity), - entity); + __bfq_entity_update_weight_prio(bfq_entity_service_tree(entity), + entity, false); } /* diff --git a/block/bfq-iosched.h b/block/bfq-iosched.h index 5c3bf98..8fd83b8 100644 --- a/block/bfq-iosched.h +++ b/block/bfq-iosched.h @@ -892,7 +892,8 @@ void bfq_put_idle_entity(struct bfq_service_tree *st, struct bfq_entity *entity); struct bfq_service_tree * __bfq_entity_update_weight_prio(struct bfq_service_tree *old_st, - struct bfq_entity *entity); + struct bfq_entity *entity, + bool update_class_too); void bfq_bfqq_served(struct bfq_queue *bfqq, int served); void bfq_bfqq_charge_time(struct bfq_data *bfqd, struct bfq_queue *bfqq, unsigned long time_ms); diff --git a/block/bfq-wf2q.c b/block/bfq-wf2q.c index 8726ede..5ec05cd 100644 --- a/block/bfq-wf2q.c +++ b/block/bfq-wf2q.c @@ -694,10 +694,28 @@ struct bfq_service_tree *bfq_entity_service_tree(struct bfq_entity *entity) return sched_data->service_tree + idx; } - +/* + * Update weight and priority of entity. If update_class_too is true, + * then update the ioprio_class of entity too. + * + * The reason why the update of ioprio_class is controlled through the + * last parameter is as follows. Changing the ioprio class of an + * entity implies changing the destination service trees for that + * entity. If such a change occurred when the entity is already on one + * of the service trees for its previous class, then the state of the + * entity would become more complex: none of the new possible service + * trees for the entity, according to bfq_entity_service_tree(), would + * match any of the possible service trees on which the entity + * is. Complex operations involving these trees, such as entity + * activations and deactivations, should take into account this + * additional complexity. To avoid this issue, this function is + * invoked with update_class_too unset in the points in the code where + * entity may happen to be on some tree. + */ struct bfq_service_tree * __bfq_entity_update_weight_prio(struct bfq_service_tree *old_st, - struct bfq_entity *entity) + struct bfq_entity *entity, + bool update_class_too) { struct bfq_service_tree *new_st = old_st; @@ -739,9 +757,15 @@ __bfq_entity_update_weight_prio(struct bfq_service_tree *old_st, bfq_weight_to_ioprio(entity->orig_weight); } - if (bfqq) + if (bfqq && update_class_too) bfqq->ioprio_class = bfqq->new_ioprio_class; - entity->prio_changed = 0; + + /* + * Reset prio_changed only if the ioprio_class change + * is not pending any longer. + */ + if (!bfqq || bfqq->ioprio_class == bfqq->new_ioprio_class) + entity->prio_changed = 0; /* * NOTE: here we may be changing the weight too early, @@ -867,7 +891,12 @@ static void bfq_update_fin_time_enqueue(struct bfq_entity *entity, { struct bfq_queue *bfqq = bfq_entity_to_bfqq(entity); - st = __bfq_entity_update_weight_prio(st, entity); + /* + * When this function is invoked, entity is not in any service + * tree, then it is safe to invoke next function with the last + * parameter set (see the comments on the function). + */ + st = __bfq_entity_update_weight_prio(st, entity, true); bfq_calc_finish(entity, entity->budget); /*