From patchwork Thu Sep 21 09:04:00 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Valente X-Patchwork-Id: 9963607 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 1B7A2600C5 for ; Thu, 21 Sep 2017 09:05:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0F43229425 for ; Thu, 21 Sep 2017 09:05:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 040D329433; Thu, 21 Sep 2017 09:05:27 +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=unavailable 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 6189229425 for ; Thu, 21 Sep 2017 09:05:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751972AbdIUJEl (ORCPT ); Thu, 21 Sep 2017 05:04:41 -0400 Received: from mail-wr0-f178.google.com ([209.85.128.178]:47707 "EHLO mail-wr0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751521AbdIUJEj (ORCPT ); Thu, 21 Sep 2017 05:04:39 -0400 Received: by mail-wr0-f178.google.com with SMTP id k20so4015482wre.4 for ; Thu, 21 Sep 2017 02:04:39 -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=uNlQFKFV+Wjj6fYjhJGqhZUyrQ/O6Mb11TIDJfMIsUY=; b=BNh5Ll53WgOWL3yr3be09uPAffWbTCRztp+QqdaAfZidzP20mnKO8b2NP0sr1ePNAN at4/X2vpEPBT5WXao3ofQw0ai6DJpElyM9/ecZYOsqDqcPljOo/qb4zczQ9/4OFom1UG A3Viep4LKvWoICFWF6X9O8aPBV5t69dyl8ZyI= 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=uNlQFKFV+Wjj6fYjhJGqhZUyrQ/O6Mb11TIDJfMIsUY=; b=CTMAUIBV2YHmzOmzE9zMEnlKlC+8fcSsggIePq2+EJYuujkG3RB7vgQOFIZguqOuVa Pv7S5DNG0oxhnMwoXeDucru/4fOHwHHo90n8Tkdz3ptolVfUyKypALYpE+zJR+0Mdsn+ NhjztrIBleW2vBvGJccsrbBxSIIHRCjW+qMHNOrgjT3ogfe1Ag6mYvHH3uQDH73P3PL5 Oi/Ws/d2vJ8r49mXFo73RuxwdOtZuA8hov9vhPcYB+3X7q9VbFW236onhrQUgg7xKWmG y8lJ19RjX9dZkYrNEmfVlX5gyteQk5R/layHXADFYlLOZ8CYI9SUxggvsZw+HrrVy/I0 g87A== X-Gm-Message-State: AHPjjUhSctMMFwdNWzzQkdPM5hTVaujgtT1yxPe8vOgSURvY+LSvRa1+ AWZnDBmq695P6Y+DMNQFoi0RKA== X-Google-Smtp-Source: AOwi7QCFmD1Tj+iNnizs7nKivETWMNJeIAtHb6X6DVHKLp5FXWl5eIMuwzunzApAM8qntczUbyWwxg== X-Received: by 10.223.182.71 with SMTP id i7mr1170202wre.43.1505984678128; Thu, 21 Sep 2017 02:04:38 -0700 (PDT) Received: from localhost.localdomain ([185.14.11.73]) by smtp.gmail.com with ESMTPSA id o59sm804032wrc.45.2017.09.21.02.04.36 (version=TLS1 cipher=AES128-SHA bits=128/128); Thu, 21 Sep 2017 02:04:37 -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, lee.tibbert@gmail.com, oleksandr@natalenko.name, mirkomontanari91@gmail.com, angeloruocco90@gmail.com, mauro.andreolini@unimore.it, Paolo Valente Subject: [PATCH BUGFIX/IMPROVEMENT 1/4] block, bfq: fix wrong init of saved start time for weight raising Date: Thu, 21 Sep 2017 11:04:00 +0200 Message-Id: <20170921090403.3217-2-paolo.valente@linaro.org> X-Mailer: git-send-email 2.10.0 In-Reply-To: <20170921090403.3217-1-paolo.valente@linaro.org> References: <20170921090403.3217-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 This commit fixes a bug that causes bfq to fail to guarantee a high responsiveness on some drives, if there is heavy random read+write I/O in the background. More precisely, such a failure allowed this bug to be found [1], but the bug may well cause other yet unreported anomalies. BFQ raises the weight of the bfq_queues associated with soft real-time applications, to privilege the I/O, and thus reduce latency, for these applications. This mechanism is named soft-real-time weight raising in BFQ. A soft real-time period may happen to be nested into an interactive weight raising period, i.e., it may happen that, when a bfq_queue switches to a soft real-time weight-raised state, the bfq_queue is already being weight-raised because deemed interactive too. In this case, BFQ saves in a special variable wr_start_at_switch_to_srt, the time instant when the interactive weight-raising period started for the bfq_queue, i.e., the time instant when BFQ started to deem the bfq_queue interactive. This value is then used to check whether the interactive weight-raising period would still be in progress when the soft real-time weight-raising period ends. If so, interactive weight raising is restored for the bfq_queue. This restore is useful, in particular, because it prevents bfq_queues from losing their interactive weight raising prematurely, as a consequence of spurious, short-lived soft real-time weight-raising periods caused by wrong detections as soft real-time. If, instead, a bfq_queue switches to soft-real-time weight raising while it *is not* already in an interactive weight-raising period, then the variable wr_start_at_switch_to_srt has no meaning during the following soft real-time weight-raising period. Unfortunately the handling of this case is wrong in BFQ: not only the variable is not flagged somehow as meaningless, but it is also set to the time when the switch to soft real-time weight-raising occurs. This may cause an interactive weight-raising period to be considered mistakenly as still in progress, and thus a spurious interactive weight-raising period to start for the bfq_queue, at the end of the soft-real-time weight-raising period. In particular the spurious interactive weight-raising period will be considered as still in progress, if the soft-real-time weight-raising period does not last very long. The bfq_queue will then be wrongly privileged and, if I/O bound, will unjustly steal bandwidth to truly interactive or soft real-time bfq_queues, harming responsiveness and low latency. This commit fixes this issue by just setting wr_start_at_switch_to_srt to minus infinity (farthest past time instant according to jiffies macros): when the soft-real-time weight-raising period ends, certainly no interactive weight-raising period will be considered as still in progress. [1] Background I/O Type: Random - Background I/O mix: Reads and writes - Application to start: LibreOffice Writer in http://www.phoronix.com/scan.php?page=news_item&px=Linux-4.13-IO-Laptop Signed-off-by: Paolo Valente Signed-off-by: Angelo Ruocco Tested-by: Oleksandr Natalenko Tested-by: Lee Tibbert Tested-by: Mirko Montanari --- block/bfq-iosched.c | 50 +++++++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index a4783da..c25955c 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -1202,6 +1202,24 @@ static unsigned int bfq_wr_duration(struct bfq_data *bfqd) return dur; } +/* + * Return the farthest future time instant according to jiffies + * macros. + */ +static unsigned long bfq_greatest_from_now(void) +{ + return jiffies + MAX_JIFFY_OFFSET; +} + +/* + * Return the farthest past time instant according to jiffies + * macros. + */ +static unsigned long bfq_smallest_from_now(void) +{ + return jiffies - MAX_JIFFY_OFFSET; +} + static void bfq_update_bfqq_wr_on_rq_arrival(struct bfq_data *bfqd, struct bfq_queue *bfqq, unsigned int old_wr_coeff, @@ -1216,7 +1234,19 @@ static void bfq_update_bfqq_wr_on_rq_arrival(struct bfq_data *bfqd, bfqq->wr_coeff = bfqd->bfq_wr_coeff; bfqq->wr_cur_max_time = bfq_wr_duration(bfqd); } else { - bfqq->wr_start_at_switch_to_srt = jiffies; + /* + * No interactive weight raising in progress + * here: assign minus infinity to + * wr_start_at_switch_to_srt, to make sure + * that, at the end of the soft-real-time + * weight raising periods that is starting + * now, no interactive weight-raising period + * may be wrongly considered as still in + * progress (and thus actually started by + * mistake). + */ + bfqq->wr_start_at_switch_to_srt = + bfq_smallest_from_now(); bfqq->wr_coeff = bfqd->bfq_wr_coeff * BFQ_SOFTRT_WEIGHT_FACTOR; bfqq->wr_cur_max_time = @@ -2897,24 +2927,6 @@ static unsigned long bfq_bfqq_softrt_next_start(struct bfq_data *bfqd, jiffies + nsecs_to_jiffies(bfqq->bfqd->bfq_slice_idle) + 4); } -/* - * Return the farthest future time instant according to jiffies - * macros. - */ -static unsigned long bfq_greatest_from_now(void) -{ - return jiffies + MAX_JIFFY_OFFSET; -} - -/* - * Return the farthest past time instant according to jiffies - * macros. - */ -static unsigned long bfq_smallest_from_now(void) -{ - return jiffies - MAX_JIFFY_OFFSET; -} - /** * bfq_bfqq_expire - expire a queue. * @bfqd: device owning the queue.