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: 9822147 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6B55260237 for ; Mon, 3 Jul 2017 08:01:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 63C8828556 for ; Mon, 3 Jul 2017 08:01:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 586EE28564; Mon, 3 Jul 2017 08:01:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8121A2856A for ; Mon, 3 Jul 2017 08:01:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752742AbdGCIBW (ORCPT ); Mon, 3 Jul 2017 04:01:22 -0400 Received: from mail-wr0-f178.google.com ([209.85.128.178]:33366 "EHLO mail-wr0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753037AbdGCIBH (ORCPT ); Mon, 3 Jul 2017 04:01:07 -0400 Received: by mail-wr0-f178.google.com with SMTP id r103so230057803wrb.0 for ; Mon, 03 Jul 2017 01:01:07 -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=Hav/tRPy7LotUj+CQSyMNhXYDgFh2iEUtwJr87QwRzCUNmsy773PlEqQGthkzxOD// mPneaeR5I+0tc2fNHL9TefAR4V/5cFexKyqyHglsEEzr3IHWKESkWbAeX3VGjQ4rEnbq mzToktIK0wAep0EBUjDgBcVZqdg0+9TovcJN/QImZ+vsTHrmjh7miB96zvwkJaqnBluG XbFrwLpW6OkvHyOmdnMZisrrMd18pf4JKkRRgwydmXjfvyHSiN4TREVg85LeYD0kK2KO 0fp2CebRhSyjEGIjCOZQBzHoXqTP2FvL8ix5pFT49h5IjqFdwQj7UwG48I7c5fs+IYVx lfOw== X-Gm-Message-State: AKS2vOxxnr1qyKHaIjyVpCpPaDJ5Q/ecoRaobmHC+C4iN0jGg3skJhb+ QcFeZd7XKapuqgc1 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-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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(-) 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); /*