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 8442FC433EF for ; Tue, 17 May 2022 07:50:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240782AbiEQHuP (ORCPT ); Tue, 17 May 2022 03:50:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43262 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236036AbiEQHuN (ORCPT ); Tue, 17 May 2022 03:50:13 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 9267544775 for ; Tue, 17 May 2022 00:50:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1652773811; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fcxa52FaP/LJSTmTsYRMIBzyizNUgowBuXRZOO4RnAA=; b=VPny+DKtQg1QfmgCXr0WbYrT3NApY27T98anfyYSZDhvv/qsGU/SSDItd9F43QV4DkT2/x ObnH0U9uH7zx02JTMifgw3IiOWg9oDDUK3P4Lf5XWJfdt0FtuBeubTvewPfQL8SYCPFGPc QDW+FafzXELWvwO1r2q+cPRhvBjUzY8= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-433-zzZy4rXjM0e348ewSnTnaA-1; Tue, 17 May 2022 03:50:10 -0400 X-MC-Unique: zzZy4rXjM0e348ewSnTnaA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id B12DE3C138A6; Tue, 17 May 2022 07:50:09 +0000 (UTC) Received: from T590 (ovpn-8-20.pek2.redhat.com [10.72.8.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 309F9154A380; Tue, 17 May 2022 07:50:03 +0000 (UTC) Date: Tue, 17 May 2022 15:49:58 +0800 From: Ming Lei To: "yukuai (C)" Cc: Tejun Heo , Zhang Wensheng , axboe@kernel.dk, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, cgroups@vger.kernel.org Subject: Re: [PATCH -next] block: fix io hung of setting throttle limit frequently Message-ID: References: <20220516014429.33723-1-zhangwensheng5@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org On Tue, May 17, 2022 at 11:12:28AM +0800, yukuai (C) wrote: > 在 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. IMO, you can't do that for iops limit. If BIO_THROTTLED is set for one bio, all its bytes have been accounted, so no need to throttle this bio in case of bps limit. iops limit is another story, since io account is done in request IO which is based on split bio, so the bio(split bio) still need to be check & throttle in case of iops limit. Thanks, Ming