From patchwork Thu May 31 13:23:12 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Valente X-Patchwork-Id: 10441087 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 B89A3603B5 for ; Thu, 31 May 2018 13:23:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9FFF829519 for ; Thu, 31 May 2018 13:23:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9464B29364; Thu, 31 May 2018 13:23:44 +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=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, MAILING_LIST_MULTI, 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 48EE028CFD for ; Thu, 31 May 2018 13:23:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755287AbeEaNXk (ORCPT ); Thu, 31 May 2018 09:23:40 -0400 Received: from mail-wr0-f193.google.com ([209.85.128.193]:38880 "EHLO mail-wr0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755268AbeEaNXf (ORCPT ); Thu, 31 May 2018 09:23:35 -0400 Received: by mail-wr0-f193.google.com with SMTP id 94-v6so32978300wrf.5 for ; Thu, 31 May 2018 06:23:35 -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=EDxxfeUlzKc3ZdpNXUqSJs2cf7e+TOKIjiRlTJbaC3c=; b=ktD10YBcp5E8ciX+3S5pI3WjJnM6xxQXu0WgI6vyFWNTOYz5LV9GEoGHkXiwZoF0Ie 2cetWMyW/dOIkOlMop3xzbFkctPqT+igg8/RToL0AdTgjktbRERMJmW9iq6Th700/IUx TbIUHvZXagH9GYrUKGS29ldj4fsrzLNWHCs8s= 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=EDxxfeUlzKc3ZdpNXUqSJs2cf7e+TOKIjiRlTJbaC3c=; b=ehnr76PhheFllb9Eu9TJhGW8VeaPkH+IASvSo7xmi92XUT+Z12E3YN8TjNX9f6h+gf ab5n6vtpPg9NB78fSA7izkdk2Kk7bvgg4jOhPTkGth3XgzhvX13iTI0jdIDGCJq/bf2y oRQhTAr1Cax+p7lWrCqMQwMh+cDsGnGRp9AsEMnKSawVpTeX7qKtNFzVrWmMamRzt5+H N4lLrmB029eRIMkbraHiJKE6LpR7NHXGJGXKoXFEcgZqAhBfs5e95CoLclgfW0hU+JZk ax4OKEwGhheKgXh7OGiTzaI+OOG8Mq/nxVQ7xuzo5rpccUeC4H8HjxqdwEOCF4Txo9iY TPfA== X-Gm-Message-State: ALKqPwdY6Vm7ic0DDkmxNfF2i90czddeMKgqQ2wmkxysPw17racXiupS IgOPmfEA+0U6V3fxoeg5596H1g== X-Google-Smtp-Source: ADUXVKKt9vSUdjxIUxRUjKOY+CdEuma4mxTUhmphvHUetvePeSn7vIeOca0pACBp1dnGEjJYyNLOcw== X-Received: by 2002:adf:8290:: with SMTP id 16-v6mr5346141wrc.38.1527773014580; Thu, 31 May 2018 06:23:34 -0700 (PDT) Received: from localhost.localdomain (146-241-12-84.dyn.eolo.it. [146.241.12.84]) by smtp.gmail.com with ESMTPSA id e133-v6sm1759478wma.38.2018.05.31.06.23.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 31 May 2018 06:23:33 -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, linus.walleij@linaro.org, bfq-iosched@googlegroups.com, oleksandr@natalenko.name, filippo.muzzini@outlook.it, Paolo Valente Subject: [PATCH BUGFIX 2/3] block, bfq: remove wrong check in bfq_requests_merged Date: Thu, 31 May 2018 15:23:12 +0200 Message-Id: <20180531132313.2986-3-paolo.valente@linaro.org> X-Mailer: git-send-email 2.16.1 In-Reply-To: <20180531132313.2986-1-paolo.valente@linaro.org> References: <20180531132313.2986-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 The request rq passed to the function bfq_requests_merged is always in a bfq_queue, so the check !RB_EMPTY_NODE(&rq->rb_node) at the beginning of bfq_requests_merged always succeeds, and the control flow systematically skips to the end of the function. This implies that the body of the function is never executed, i.e., the repositioning of rq is never performed. On the opposite end, a control is missing in the body of the function: 'next' must be removed only if it is inside a bfq_queue. This commit removes the wrong check on rq, and adds the missing check on 'next'. In addition, this commit adds comments on bfq_requests_merged. Signed-off-by: Filippo Muzzini Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index 1f0951d36424..df2a9633cf4a 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -1891,14 +1891,27 @@ static void bfq_request_merged(struct request_queue *q, struct request *req, } } +/* + * This function is called to notify the scheduler that the requests + * rq and 'next' have been merged, with 'next' going away. BFQ + * exploits this hook to address the following issue: if 'next' has a + * fifo_time lower that rq, then the fifo_time of rq must be set to + * the value of 'next', to not forget the greater age of 'next'. + * Moreover 'next' may be in a bfq_queue, in this case it must be + * removed. + * + * NOTE: in this function we assume that rq is in a bfq_queue, basing + * on that rq is picked from the hash table q->elevator->hash, which, + * in its turn, is filled only with I/O requests present in + * bfq_queues, while BFQ is in use for the request queue q. In fact, + * the function that fills this hash table (elv_rqhash_add) is called + * only by bfq_insert_request. + */ static void bfq_requests_merged(struct request_queue *q, struct request *rq, struct request *next) { struct bfq_queue *bfqq = RQ_BFQQ(rq), *next_bfqq = RQ_BFQQ(next); - if (!RB_EMPTY_NODE(&rq->rb_node)) - goto end; - /* * If next and rq belong to the same bfq_queue and next is older * than rq, then reposition rq in the fifo (by substituting next @@ -1919,10 +1932,11 @@ static void bfq_requests_merged(struct request_queue *q, struct request *rq, if (bfqq->next_rq == next) bfqq->next_rq = rq; - bfq_remove_request(q, next); - bfqg_stats_update_io_remove(bfqq_group(bfqq), next->cmd_flags); + if (!RB_EMPTY_NODE(&next->rb_node)) { + bfq_remove_request(q, next); + bfqg_stats_update_io_remove(bfqq_group(bfqq), next->cmd_flags); + } -end: bfqg_stats_update_io_merged(bfqq_group(bfqq), next->cmd_flags); }