From patchwork Mon Nov 13 20:37:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 10056671 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 E7EE9602A7 for ; Mon, 13 Nov 2017 20:37:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DA59D29316 for ; Mon, 13 Nov 2017 20:37:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CF04129546; Mon, 13 Nov 2017 20:37:09 +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=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID 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 7775729316 for ; Mon, 13 Nov 2017 20:37:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755558AbdKMUhI (ORCPT ); Mon, 13 Nov 2017 15:37:08 -0500 Received: from mail-qt0-f194.google.com ([209.85.216.194]:55662 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755361AbdKMUhH (ORCPT ); Mon, 13 Nov 2017 15:37:07 -0500 Received: by mail-qt0-f194.google.com with SMTP id v41so21198469qtv.12 for ; Mon, 13 Nov 2017 12:37:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=nFxXwuFCA8+oVfbADCOmBVnBW+fwUNzy9XvPTKy7lGg=; b=DefmY8jsGuVT1u5OzuJuTE8XQtwa8tV5dH3m2r/8KSBgZiKLY8H6ZniINLMHlGLw7f N4qQYSQdErCBl8TaBL9Xl3pKAW98KvONpTI3hXy/FAV8mfpT1yl6bBQN+jhEFwZsiP8h GaEQXhShGBRi67wUvFE1ehDOfatrCUqO1jOdo2THhnFkIckSzb8v3TNkyH67dtAb/IM3 07/Vo1zzxDskHigdj2ez/P4z5BMUSfhppTK0eUghkB4KiUuI1MZgXlUyVDeB1O8iJDSr h6qq/sBAErkfabPb5fBVnaqj2OEV/8TFWhps4QKow1MZTHi2rachcyUbq5gs+AMl/ch8 /zMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent; bh=nFxXwuFCA8+oVfbADCOmBVnBW+fwUNzy9XvPTKy7lGg=; b=XUMRPEyI5Jtbh0hwDMXuU2v3zhi6l1K+QGVHDBM1WDAoVsxAZTXegoXFRLxr/tUEtc y6nvH+S+HupAvFcS/LGF9UKZyNFGQfBXYIpDteDlVHLqj+6l58e7hgeJ7lDWhXze9ASF ZIISGQSD5rAhDps5Wgpb4tXhYIG8/x/tfBkPejuNnR+9pYQgi8aKckkmUTr9BJ0TX9hN WxtffOO5A6odRkG4tBuas+n41XcnHzhyuXhmsQkoA99B+C7qVaViavrwqmEJK2eVhpik nZETmh1YuPO65lgPq0/2Mpqd8PDalJzLDICOILr/EMeFENv1wcMjlyO7H6r4XU3IN9ZD +nbw== X-Gm-Message-State: AJaThX5AvDeJbIXnZGlKks4KMO9fqvKoitnXt9++Ix4GmaVI6VKnBmyF 9mojPaM/zaMhrI+qtngNjdI= X-Google-Smtp-Source: AGs4zMbfNeMNWG8I5A7kmBgFFpqE+0iFhtVIIgw5ux1j8EkqzGRrRLYeVATXuPtPXIku00TPlI5OLw== X-Received: by 10.200.24.185 with SMTP id s54mr12330876qtj.285.1510605426720; Mon, 13 Nov 2017 12:37:06 -0800 (PST) Received: from localhost (dhcp-ec-8-6b-ed-7a-cf.cpe.echoes.net. [72.28.5.223]) by smtp.gmail.com with ESMTPSA id e190sm11323555qkf.55.2017.11.13.12.37.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 13 Nov 2017 12:37:06 -0800 (PST) Date: Mon, 13 Nov 2017 12:37:03 -0800 From: Tejun Heo To: Shaohua Li Cc: linux-block@vger.kernel.org, axboe@kernel.dk, Kernel-team@fb.com, Vivek Goyal Subject: Re: [PATCH] block-throttle: avoid double charge Message-ID: <20171113203703.GP983427@devbig577.frc2.facebook.com> References: <20171113200338.GK983427@devbig577.frc2.facebook.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20171113200338.GK983427@devbig577.frc2.facebook.com> User-Agent: Mutt/1.5.21 (2010-09-15) 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 Mon, Nov 13, 2017 at 12:03:38PM -0800, Tejun Heo wrote: > So, one question I have is whether we need both BIO_THROTTLED and > bi_throttled_disk. Can't we replace BIO_THROTTLED w/ > bi_throttled_disk? IOW, won't something like the following work? (not tested yet) Thanks. --- block/bio.c | 3 +++ block/blk-throttle.c | 20 +++++++------------- include/linux/blk_types.h | 4 ++++ 3 files changed, 14 insertions(+), 13 deletions(-) --- a/block/bio.c +++ b/block/bio.c @@ -597,6 +597,9 @@ void __bio_clone_fast(struct bio *bio, s * so we don't set nor calculate new physical/hw segment counts here */ bio->bi_disk = bio_src->bi_disk; +#ifdef CONFIG_BLK_DEV_THROTTLING + bio->bi_throttled_disk = bio_src->bi_throttled_disk; +#endif bio_set_flag(bio, BIO_CLONED); bio->bi_opf = bio_src->bi_opf; bio->bi_write_hint = bio_src->bi_write_hint; --- a/block/blk-throttle.c +++ b/block/blk-throttle.c @@ -1051,13 +1051,12 @@ static void throtl_charge_bio(struct thr tg->last_io_disp[rw]++; /* - * BIO_THROTTLED is used to prevent the same bio to be throttled + * bi_throttled_disk is used to prevent the same bio to be throttled * more than once as a throttled bio will go through blk-throtl the * second time when it eventually gets issued. Set it when a bio * is being charged to a tg. */ - if (!bio_flagged(bio, BIO_THROTTLED)) - bio_set_flag(bio, BIO_THROTTLED); + bio->bi_throttled_disk = bio->bi_disk; } /** @@ -2131,8 +2130,11 @@ bool blk_throtl_bio(struct request_queue WARN_ON_ONCE(!rcu_read_lock_held()); - /* see throtl_charge_bio() */ - if (bio_flagged(bio, BIO_THROTTLED) || !tg->has_rules[rw]) + /* + * See throtl_charge_bio(). If a bio is throttled against a disk + * but remapped to other disk, we should throttle it again + */ + if (bio->bi_throttled_disk == bio->bi_disk || !tg->has_rules[rw]) goto out; spin_lock_irq(q->queue_lock); @@ -2223,14 +2225,6 @@ again: out_unlock: spin_unlock_irq(q->queue_lock); out: - /* - * As multiple blk-throtls may stack in the same issue path, we - * don't want bios to leave with the flag set. Clear the flag if - * being issued. - */ - if (!throttled) - bio_clear_flag(bio, BIO_THROTTLED); - #ifdef CONFIG_BLK_DEV_THROTTLING_LOW if (throttled || !td->track_bio_latency) bio->bi_issue_stat.stat |= SKIP_LATENCY; --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -90,6 +90,10 @@ struct bio { void *bi_cg_private; struct blk_issue_stat bi_issue_stat; #endif +#ifdef CONFIG_BLK_DEV_THROTTLING + /* record which disk the bio is throttled against */ + struct gendisk *bi_throttled_disk; +#endif #endif union { #if defined(CONFIG_BLK_DEV_INTEGRITY)