From patchwork Tue Nov 28 09:37:34 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Valente X-Patchwork-Id: 119815 Delivered-To: patch@linaro.org Received: by 10.140.22.227 with SMTP id 90csp1469629qgn; Tue, 28 Nov 2017 01:38:02 -0800 (PST) X-Google-Smtp-Source: AGs4zMY949hr6aL5de4x/ZIapfFzcas8MVU4R3oUsf8FaEsNE0m1vEppeNhGG2fkp0rlgAo7q5Lc X-Received: by 10.98.31.142 with SMTP id l14mr40247044pfj.62.1511861881919; Tue, 28 Nov 2017 01:38:01 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1511861881; cv=none; d=google.com; s=arc-20160816; b=ILRMfrjeKZCnDI3A+sP/YqBhD9NlJ4qTLKwx/nEKbdI+9mtKjyBJXdEp9hlr+t8EU5 duIIRlTKFoiCghySZOfa7zc9d07IuYzSYLWo1bjwKQXyjKrWlqzY+iQvtMkuiES4FGEf lWUFxxhgujHccfVCUg/ij+seUUteENHeEnlZ3zDJVQsfswbE57Q6tTMRj9H/IjXqfaKf NNHphtDC5bwiXSJuFtOC0Nz+CUmSa07ldidsV7WO25lGQhuLz+WFOA88V2L6mFNOX7Cl 5/nmAi6LxaOrMetaAoDVAYflFpH7nnzG+3v5wLTODabza8/At4oRi2qe1znWauIRmO89 0qTA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :dkim-signature:arc-authentication-results; bh=PXENCPMstPZa0u7Hgj8hVXwQ2dgyX5Gynbm1a3Va5YI=; b=jvrI1rLfULsT5Vj/mYhB1QOLqjD7cfhRmyNXDHl1k5j5SALe61Kw3FwrK3AtehTcOU 1CRqWtYNXP27r/Wpc1vx0OLMsue5j/2ouXPuqOq/qeoa3HIkkFljMFHXrMylxWil3ZQ1 4U/Xku7cnfoEyRwgoDtsQr/wpg8crfPhPXdNrRZHt/alOY9H8XVuGSsfPch5PQA6Oqlr cghndIAGTGgtsEOpGwoMgezbRzg1Ad77dOr0QgZjSyC301HZUNmjCxsdn9L7v6flpYI7 rgJ9PsPp30Wwnp/RSyjOFNpTprPjbdsCHCDr5hneaDqk17xvoJDsuOt0iSzO1EHU0Z1v 9B/Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=iHswsVy/; 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 s7si24088525pgq.419.2017.11.28.01.38.01; Tue, 28 Nov 2017 01:38:01 -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=iHswsVy/; 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 S1752372AbdK1Jh6 (ORCPT + 28 others); Tue, 28 Nov 2017 04:37:58 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:43386 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752109AbdK1Jh4 (ORCPT ); Tue, 28 Nov 2017 04:37:56 -0500 Received: by mail-wm0-f67.google.com with SMTP id x63so412762wmf.2 for ; Tue, 28 Nov 2017 01:37:55 -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; bh=PXENCPMstPZa0u7Hgj8hVXwQ2dgyX5Gynbm1a3Va5YI=; b=iHswsVy/q7VE70+/SfozYjClwlIxVoLBN1Tzcv+wKdNHdeyQ2/o3nB6NuB7eJZbktF KGUNXXHBpX9Rr0/lwtCyjLYGpmmSNuYqIECttmoAqtQk0b3b8Eyv9Ut6TiEd+UA8z3PG rK5ZPTbB316w82Xn6VpcnJQMGTXn5ImQM+g34= 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; bh=PXENCPMstPZa0u7Hgj8hVXwQ2dgyX5Gynbm1a3Va5YI=; b=NSGuWYL/U9lvTq/dSrcm1DIio5bg/4izA7p0T+TLoz64GaaRib64AhRbSLzgOtDc6/ 5Zz3C+Qd8DMWF6+LIKIEJxXnH5WPgDkwnpGt9Or1dqXXZY6Tw3jIf5BuIjtDeTmXG7IZ NJGmqqGeGFHR7L9CZ0ksRHPXXHMdP3wsUP2v9jN+m6j/az8HVPCkQVo6FN2IYRcTrfeX DOwl6UIH6dpXYiFxtKQJEhBcOqX++r6xFZqhg/LmhdjGP1y760WbmrRv5yHrXRp4RW7a lZRSm+FqA9/TW1C9MOiCiPTpBse9suY9UThOlopDvps4B2PjdHmawW8Tpo+YtriAklx6 kMOw== X-Gm-Message-State: AJaThX5I8ycryYtQB0yUblVHRmMhBOvBlLQfF6CG8SNHYOcyBBqsbYaJ f5RO7a90xu6C9byI2WXX9HwPrQ== X-Received: by 10.28.191.80 with SMTP id p77mr21098021wmf.85.1511861874731; Tue, 28 Nov 2017 01:37:54 -0800 (PST) Received: from localhost.localdomain ([185.14.8.76]) by smtp.gmail.com with ESMTPSA id o135sm12403687wmg.1.2017.11.28.01.37.52 (version=TLS1 cipher=AES128-SHA bits=128/128); Tue, 28 Nov 2017 01:37:53 -0800 (PST) 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, lucmiccio@gmail.com, bfq-iosched@googlegroups.com, Paolo Valente Subject: [PATCH] block, bfq: remove batches of confusing ifdefs Date: Tue, 28 Nov 2017 10:37:34 +0100 Message-Id: <20171128093734.1918-1-paolo.valente@linaro.org> X-Mailer: git-send-email 2.10.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Commit a33801e8b473 ("block, bfq: move debug blkio stats behind CONFIG_DEBUG_BLK_CGROUP") introduced two batches of confusing ifdefs: one reported in [1], plus a similar one in another function. This commit removes both batches, in the way suggested in [1]. [1] https://www.spinics.net/lists/linux-block/msg20043.html Fixes: a33801e8b473 ("block, bfq: move debug blkio stats behind CONFIG_DEBUG_BLK_CGROUP") Reported-by: Linus Torvalds Tested-by: Luca Miccio Signed-off-by: Paolo Valente --- block/bfq-iosched.c | 127 +++++++++++++++++++++++++++++----------------------- 1 file changed, 72 insertions(+), 55 deletions(-) -- 2.10.0 Reviewed-by: Ulf Hansson diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index bcb6d21..0153fea 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -3689,35 +3689,16 @@ static struct request *__bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) return rq; } -static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) -{ - struct bfq_data *bfqd = hctx->queue->elevator->elevator_data; - struct request *rq; -#if defined(CONFIG_BFQ_GROUP_IOSCHED) && defined(CONFIG_DEBUG_BLK_CGROUP) - struct bfq_queue *in_serv_queue, *bfqq; - bool waiting_rq, idle_timer_disabled; -#endif - - spin_lock_irq(&bfqd->lock); - #if defined(CONFIG_BFQ_GROUP_IOSCHED) && defined(CONFIG_DEBUG_BLK_CGROUP) - in_serv_queue = bfqd->in_service_queue; - waiting_rq = in_serv_queue && bfq_bfqq_wait_request(in_serv_queue); - - rq = __bfq_dispatch_request(hctx); - - idle_timer_disabled = - waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); - -#else - rq = __bfq_dispatch_request(hctx); -#endif - spin_unlock_irq(&bfqd->lock); +static inline void bfq_update_dispatch_stats(struct request *rq, + spinlock_t *queue_lock, + struct bfq_queue *in_serv_queue, + bool idle_timer_disabled) +{ + struct bfq_queue *bfqq = rq ? RQ_BFQQ(rq) : NULL; -#if defined(CONFIG_BFQ_GROUP_IOSCHED) && defined(CONFIG_DEBUG_BLK_CGROUP) - bfqq = rq ? RQ_BFQQ(rq) : NULL; if (!idle_timer_disabled && !bfqq) - return rq; + return; /* * rq and bfqq are guaranteed to exist until this function @@ -3732,7 +3713,7 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) * In addition, the following queue lock guarantees that * bfqq_group(bfqq) exists as well. */ - spin_lock_irq(hctx->queue->queue_lock); + spin_lock_irq(queue_lock); if (idle_timer_disabled) /* * Since the idle timer has been disabled, @@ -3751,9 +3732,37 @@ static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) bfqg_stats_set_start_empty_time(bfqg); bfqg_stats_update_io_remove(bfqg, rq->cmd_flags); } - spin_unlock_irq(hctx->queue->queue_lock); + spin_unlock_irq(queue_lock); +} +#else +static inline void bfq_update_dispatch_stats(struct request *rq, + spinlock_t *queue_lock, + struct bfq_queue *in_serv_queue, + bool idle_timer_disabled) {} #endif +static struct request *bfq_dispatch_request(struct blk_mq_hw_ctx *hctx) +{ + struct bfq_data *bfqd = hctx->queue->elevator->elevator_data; + struct request *rq; + struct bfq_queue *in_serv_queue; + bool waiting_rq, idle_timer_disabled; + + spin_lock_irq(&bfqd->lock); + + in_serv_queue = bfqd->in_service_queue; + waiting_rq = in_serv_queue && bfq_bfqq_wait_request(in_serv_queue); + + rq = __bfq_dispatch_request(hctx); + + idle_timer_disabled = + waiting_rq && !bfq_bfqq_wait_request(in_serv_queue); + + spin_unlock_irq(&bfqd->lock); + + bfq_update_dispatch_stats(rq, hctx->queue->queue_lock, in_serv_queue, + idle_timer_disabled); + return rq; } @@ -4276,16 +4285,46 @@ static bool __bfq_insert_request(struct bfq_data *bfqd, struct request *rq) return idle_timer_disabled; } +#if defined(CONFIG_BFQ_GROUP_IOSCHED) && defined(CONFIG_DEBUG_BLK_CGROUP) +static inline void bfq_update_insert_stats(struct bfq_queue *bfqq, + spinlock_t *queue_lock, + bool idle_timer_disabled, + unsigned int cmd_flags) +{ + if (!bfqq) + return; + + /* + * bfqq still exists, because it can disappear only after + * either it is merged with another queue, or the process it + * is associated with exits. But both actions must be taken by + * the same process currently executing this flow of + * instructions. + * + * In addition, the following queue lock guarantees that + * bfqq_group(bfqq) exists as well. + */ + spin_lock_irq(queue_lock); + bfqg_stats_update_io_add(bfqq_group(bfqq), bfqq, cmd_flags); + if (idle_timer_disabled) + bfqg_stats_update_idle_time(bfqq_group(bfqq)); + spin_unlock_irq(queue_lock); +} +#else +static inline void bfq_update_insert_stats(struct bfq_queue *bfqq, + spinlock_t *queue_lock, + bool idle_timer_disabled, + unsigned int cmd_flags) {} +#endif + static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, bool at_head) { struct request_queue *q = hctx->queue; struct bfq_data *bfqd = q->elevator->elevator_data; -#if defined(CONFIG_BFQ_GROUP_IOSCHED) && defined(CONFIG_DEBUG_BLK_CGROUP) struct bfq_queue *bfqq = RQ_BFQQ(rq); bool idle_timer_disabled = false; unsigned int cmd_flags; -#endif spin_lock_irq(&bfqd->lock); if (blk_mq_sched_try_insert_merge(q, rq)) { @@ -4304,7 +4343,6 @@ static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, else list_add_tail(&rq->queuelist, &bfqd->dispatch); } else { -#if defined(CONFIG_BFQ_GROUP_IOSCHED) && defined(CONFIG_DEBUG_BLK_CGROUP) idle_timer_disabled = __bfq_insert_request(bfqd, rq); /* * Update bfqq, because, if a queue merge has occurred @@ -4312,9 +4350,6 @@ static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, * redirected into a new queue. */ bfqq = RQ_BFQQ(rq); -#else - __bfq_insert_request(bfqd, rq); -#endif if (rq_mergeable(rq)) { elv_rqhash_add(q, rq); @@ -4323,35 +4358,17 @@ static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq, } } -#if defined(CONFIG_BFQ_GROUP_IOSCHED) && defined(CONFIG_DEBUG_BLK_CGROUP) /* * Cache cmd_flags before releasing scheduler lock, because rq * may disappear afterwards (for example, because of a request * merge). */ cmd_flags = rq->cmd_flags; -#endif + spin_unlock_irq(&bfqd->lock); -#if defined(CONFIG_BFQ_GROUP_IOSCHED) && defined(CONFIG_DEBUG_BLK_CGROUP) - if (!bfqq) - return; - /* - * bfqq still exists, because it can disappear only after - * either it is merged with another queue, or the process it - * is associated with exits. But both actions must be taken by - * the same process currently executing this flow of - * instruction. - * - * In addition, the following queue lock guarantees that - * bfqq_group(bfqq) exists as well. - */ - spin_lock_irq(q->queue_lock); - bfqg_stats_update_io_add(bfqq_group(bfqq), bfqq, cmd_flags); - if (idle_timer_disabled) - bfqg_stats_update_idle_time(bfqq_group(bfqq)); - spin_unlock_irq(q->queue_lock); -#endif + bfq_update_insert_stats(bfqq, q->queue_lock, idle_timer_disabled, + cmd_flags); } static void bfq_insert_requests(struct blk_mq_hw_ctx *hctx,