From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 22DAEC433EF for ; Tue, 17 May 2022 03:12:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238768AbiEQDMl (ORCPT ); Mon, 16 May 2022 23:12:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56320 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239781AbiEQDMi (ORCPT ); Mon, 16 May 2022 23:12:38 -0400 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 54B1E1EC58; Mon, 16 May 2022 20:12:32 -0700 (PDT) Received: from kwepemi500015.china.huawei.com (unknown [172.30.72.53]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4L2Ljp3wbHz1JC5q; Tue, 17 May 2022 11:11:10 +0800 (CST) Received: from kwepemm600009.china.huawei.com (7.193.23.164) by kwepemi500015.china.huawei.com (7.221.188.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 17 May 2022 11:12:30 +0800 Received: from [10.174.176.73] (10.174.176.73) by kwepemm600009.china.huawei.com (7.193.23.164) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 17 May 2022 11:12:29 +0800 Subject: Re: [PATCH -next] block: fix io hung of setting throttle limit frequently To: Tejun Heo , Zhang Wensheng , "ming.lei@redhat.com >> Ming Lei" CC: , , , References: <20220516014429.33723-1-zhangwensheng5@huawei.com> From: "yukuai (C)" Message-ID: Date: Tue, 17 May 2022 11:12:28 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="gbk"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.176.73] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To kwepemm600009.china.huawei.com (7.193.23.164) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org ÔÚ 2022/05/17 3:29, Tejun Heo дµÀ: > On Mon, May 16, 2022 at 09:44:29AM +0800, Zhang Wensheng wrote: >> diff --git a/block/blk-throttle.c b/block/blk-throttle.c >> index 469c483719be..8acb205dfa85 100644 >> --- a/block/blk-throttle.c >> +++ b/block/blk-throttle.c >> @@ -1321,12 +1321,14 @@ static void tg_conf_updated(struct throtl_grp *tg, bool global) >> * that a group's limit are dropped suddenly and we don't want to >> * account recently dispatched IO with new low rate. >> */ >> - throtl_start_new_slice(tg, READ); >> - throtl_start_new_slice(tg, WRITE); >> + if (!timer_pending(&sq->parent_sq->pending_timer)) { >> + throtl_start_new_slice(tg, READ); >> + throtl_start_new_slice(tg, WRITE); >> >> - if (tg->flags & THROTL_TG_PENDING) { >> - tg_update_disptime(tg); >> - throtl_schedule_next_dispatch(sq->parent_sq, true); >> + if (tg->flags & THROTL_TG_PENDING) { >> + tg_update_disptime(tg); >> + throtl_schedule_next_dispatch(sq->parent_sq, true); >> + } > > Yeah, but this ends up breaking the reason why it's starting the new slices > in the first place explained in the commit above, right? I'm not sure what > the right solution is but this likely isn't it. > Hi, Tejun Ming added a condition in tg_with_in_bps_limit(): - if (bps_limit == U64_MAX) { + /* no need to throttle if this bio's bytes have been accounted */ + if (bps_limit == U64_MAX || bio_flagged(bio, BIO_THROTTLED)) { Which will let the first throttled bio to be issued immediately once the config if updated. Do you think this behaviour is OK? If so, we can do the same for tg_with_in_iops_limit. Thanks, Kuai > From mboxrd@z Thu Jan 1 00:00:00 1970 From: "yukuai (C)" Subject: Re: [PATCH -next] block: fix io hung of setting throttle limit frequently Date: Tue, 17 May 2022 11:12:28 +0800 Message-ID: References: <20220516014429.33723-1-zhangwensheng5@huawei.com> Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-ID: Content-Type: text/plain; charset="iso-8859-1"; format="flowed" To: Tejun Heo , Zhang Wensheng , "ming.lei@redhat.com >> Ming Lei" Cc: axboe@kernel.dk, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org =D4=DA 2022/05/17 3:29, Tejun Heo =D0=B4=B5=C0: > On Mon, May 16, 2022 at 09:44:29AM +0800, Zhang Wensheng wrote: >> diff --git a/block/blk-throttle.c b/block/blk-throttle.c >> index 469c483719be..8acb205dfa85 100644 >> --- a/block/blk-throttle.c >> +++ b/block/blk-throttle.c >> @@ -1321,12 +1321,14 @@ static void tg_conf_updated(struct throtl_grp *t= g, bool global) >> * that a group's limit are dropped suddenly and we don't want to >> * account recently dispatched IO with new low rate. >> */ >> - throtl_start_new_slice(tg, READ); >> - throtl_start_new_slice(tg, WRITE); >> + if (!timer_pending(&sq->parent_sq->pending_timer)) { >> + throtl_start_new_slice(tg, READ); >> + throtl_start_new_slice(tg, WRITE); >> =20 >> - if (tg->flags & THROTL_TG_PENDING) { >> - tg_update_disptime(tg); >> - throtl_schedule_next_dispatch(sq->parent_sq, true); >> + if (tg->flags & THROTL_TG_PENDING) { >> + tg_update_disptime(tg); >> + throtl_schedule_next_dispatch(sq->parent_sq, true); >> + } >=20 > Yeah, but this ends up breaking the reason why it's starting the new slic= es > in the first place explained in the commit above, right? I'm not sure what > the right solution is but this likely isn't it. >=20 Hi, Tejun Ming added a condition in tg_with_in_bps_limit(): - if (bps_limit =3D=3D U64_MAX) { + /* no need to throttle if this bio's bytes have been accounted */ + if (bps_limit =3D=3D U64_MAX || bio_flagged(bio, BIO_THROTTLED)) { Which will let the first throttled bio to be issued immediately once the config if updated. Do you think this behaviour is OK? If so, we can do the same for tg_with_in_iops_limit. Thanks, Kuai >=20