From patchwork Mon Dec 4 10:42:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Valente X-Patchwork-Id: 120507 Delivered-To: patch@linaro.org Received: by 10.140.22.227 with SMTP id 90csp4260563qgn; Mon, 4 Dec 2017 02:42:16 -0800 (PST) X-Google-Smtp-Source: AGs4zMZr//mPyYoYdClYLVMveWIteNrNLH/ZryTMcvmsmefuJiFcXUn7C3mCZqNMj+yTjwookvOP X-Received: by 10.99.117.83 with SMTP id f19mr13474472pgn.225.1512384136254; Mon, 04 Dec 2017 02:42:16 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1512384136; cv=none; d=google.com; s=arc-20160816; b=YGU23K+u4Yo/amx0WUyW90TUfPya0+QoFNxJYKPLxemjMdGoZQbuZDP84P1/99x55b n6cylIT+dF2XRBuchHW4+CXRbX9fdajePl7tdHwfDCwy88HOYdvO9Jt8iC2sF2Q44+Zl 6FTXRDahNXEd+W7DICN+/zdHw44gZ7NE+KWIxqwlzQAieQMLuyorqq0VWlSarOUv45l2 OWtgGQ/nVJAC+2kFL47dCelBpZLTwikYuH5wDVTBKDeJn7b0UP1HjN39n8XPBL0BYhEf REx0EJR0/9OG2nm07As8WwmW/JiGjogczkRd0u4eb16DDekVvlgNTJD6ZzL266UhTuZg FfcQ== 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=IIJV6/CpZoqnHosKw+SQFI0XPAfm6/f4QMeImGCIGOg=; b=g3itDIYlrJVRpSb6jHyIlcufDzc01oyWuIplKcg7gEo/S/fJye44DH/lqF0ckebSMm YKaoYqZGfOmCVvmgBl4sfGojAtUHD+3XGzgu36bF/No8DafXvlpVh8zs9MjhkzSPo3F5 8iNgY1p29QQhR4DRQiLdKYgaWxosmQBcHPONt0jVHYK6MXWHVRc+7KgeoxTspVx827WS at0iiP6vogc2gZERK5EgyStTxL0glXBiMvXDJu5Y7l+icMPZ/QX+ZGicPBT+NLA5hyS4 vjv1RtDg9/ELYCbBcAEFOOScQkTkGHwLyP+y5HKHkVkMYzrobbvUK9KGZ/yvB3VeqCkt ShSg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=ifCul4rj; 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 f9si9278388plo.763.2017.12.04.02.42.15; Mon, 04 Dec 2017 02:42:16 -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=ifCul4rj; 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 S1753770AbdLDKmN (ORCPT + 28 others); Mon, 4 Dec 2017 05:42:13 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:44594 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753692AbdLDKmL (ORCPT ); Mon, 4 Dec 2017 05:42:11 -0500 Received: by mail-wm0-f67.google.com with SMTP id t8so4883779wmc.3 for ; Mon, 04 Dec 2017 02:42:10 -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=IIJV6/CpZoqnHosKw+SQFI0XPAfm6/f4QMeImGCIGOg=; b=ifCul4rjA2YZKvLr5xMZbE7SeiHqDVUCN4UROmGiwGVcF7NtHc50KjStIJKjtew8Oz jNemj8WLCUD8sNJdcskYhXdI+I0jRlgmGCRjcrWKXijWe34OP9bwJhdu8z13+/djm3pf zF9Do+bLhwkpbDYvA134vT047OgMyG1uHY89E= 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=IIJV6/CpZoqnHosKw+SQFI0XPAfm6/f4QMeImGCIGOg=; b=b61CXdjJyVWcwqDzh8u/L/3pcF/8l31OB3r+DrzhiEYwGKrSFmJvZzbOlEVvDnEfG9 NqNClLSwkTzidcFXXTGYNIZxpoApRH5QxChU0pruTZmpgmXB4OQ0MGWVdS39/kX9VMEQ KqK95pH73HqUOya9n6MOWw09vh4xbUnvsxHhMcKjIqVZSU1i7eB/FCVjE4ZtRoiFYXZK jMMIeKhdkjnfpFi7vjEscvQRVyoPZG8LjpnPAiRVlo5kNQaACes2GXDLQ+dUrMwe5lNY gbS/gyBjwDj831fWucswNzXsHqIV8wKMgVgGPoUqKQKmV1soQZF046DlKLQp6LjJC3cI 9zKg== X-Gm-Message-State: AKGB3mI4Nrm4fFhqbfz9lOvxyxIYGa43qNtWqJ2bno7j0k4L+6pC7lus EdOOPvX1EPImlgyzAiN18831Jea9RNg= X-Received: by 10.28.129.77 with SMTP id c74mr2742574wmd.124.1512384130080; Mon, 04 Dec 2017 02:42:10 -0800 (PST) Received: from wifi-122_dhcprange-87.wifi.unimo.it ([155.185.122.87]) by smtp.gmail.com with ESMTPSA id w46sm11962562wrb.86.2017.12.04.02.42.08 (version=TLS1 cipher=AES128-SHA bits=128/128); Mon, 04 Dec 2017 02:42:08 -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, Paolo Valente Subject: [PATCH V2] block, bfq: remove batches of confusing ifdefs Date: Mon, 4 Dec 2017 11:42:05 +0100 Message-Id: <20171204104205.3290-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 diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index bcb6d21..3feed64 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 void bfq_update_dispatch_stats(struct request_queue *q, + struct request *rq, + 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(q->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(q->queue_lock); +} +#else +static inline void bfq_update_dispatch_stats(struct request_queue *q, + struct request *rq, + 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(hctx->queue, rq, 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 void bfq_update_insert_stats(struct request_queue *q, + struct bfq_queue *bfqq, + 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(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); +} +#else +static inline void bfq_update_insert_stats(struct request_queue *q, + struct bfq_queue *bfqq, + 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(q, bfqq, idle_timer_disabled, + cmd_flags); } static void bfq_insert_requests(struct blk_mq_hw_ctx *hctx,