From patchwork Wed Apr 27 15:21:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jens Axboe X-Patchwork-Id: 8958981 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 6B44A9F441 for ; Wed, 27 Apr 2016 15:22:45 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E82B520260 for ; Wed, 27 Apr 2016 15:22:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 674B320259 for ; Wed, 27 Apr 2016 15:22:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753140AbcD0PWU (ORCPT ); Wed, 27 Apr 2016 11:22:20 -0400 Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:28828 "EHLO mx0b-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752764AbcD0PWT (ORCPT ); Wed, 27 Apr 2016 11:22:19 -0400 Received: from pps.filterd (m0001255.ppops.net [127.0.0.1]) by mx0b-00082601.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u3RFJCsD003504; Wed, 27 Apr 2016 08:21:55 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=fb.com; h=subject : to : references : cc : from : message-id : date : mime-version : in-reply-to : content-type; s=facebook; bh=d/3SVXfD+j9dOywbes+uEq2MdueW3fkzlfRChk+1Hsk=; b=j+mV5Zv+9OVF+if28jQawanN7S5ywrNZczvqoGO3FX15aK4RZgrkyJ/FKFLlKcse0d2x KtpWBTkxul9mxJflwL7/JzDG+mQOg5kiP9Jy71u5IXtReC9ig+qKMYIUjrrzPdcRn0y/ CEe5OKy6mAPqcO9Jk9P9zE2NKAiDjUjxI2Y= Received: from mail.thefacebook.com ([199.201.64.23]) by mx0b-00082601.pphosted.com with ESMTP id 22ju889vbq-1 (version=TLSv1 cipher=ECDHE-RSA-AES256-SHA bits=256 verify=NOT); Wed, 27 Apr 2016 08:21:55 -0700 Received: from [IPv6:2620:10d:c081:1103:1983:3723:c421:d2b5] (192.168.52.123) by mail.thefacebook.com (192.168.16.15) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 27 Apr 2016 08:21:53 -0700 Subject: Re: [PATCH 7/8] wbt: add general throttling mechanism To: xiakaixu References: <1461686131-22999-1-git-send-email-axboe@fb.com> <1461686131-22999-8-git-send-email-axboe@fb.com> <5720AB61.8010109@huawei.com> CC: , , , , , , "miaoxie (A)" , Huxinwei , Bintian From: Jens Axboe Message-ID: <5720D90F.6000609@fb.com> Date: Wed, 27 Apr 2016 09:21:51 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <5720AB61.8010109@huawei.com> X-Originating-IP: [192.168.52.123] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2016-04-27_08:, , signatures=0 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 04/27/2016 06:06 AM, xiakaixu wrote: >> +void __wbt_done(struct rq_wb *rwb) >> +{ >> + int inflight, limit = rwb->wb_normal; >> + >> + /* >> + * If the device does write back caching, drop further down >> + * before we wake people up. >> + */ >> + if (rwb->wc && !atomic_read(&rwb->bdi->wb.dirty_sleeping)) >> + limit = 0; >> + else >> + limit = rwb->wb_normal; >> + >> + /* >> + * Don't wake anyone up if we are above the normal limit. If >> + * throttling got disabled (limit == 0) with waiters, ensure >> + * that we wake them up. >> + */ >> + inflight = atomic_dec_return(&rwb->inflight); >> + if (limit && inflight >= limit) { >> + if (!rwb->wb_max) >> + wake_up_all(&rwb->wait); >> + return; >> + } >> + > Hi Jens, > > Just a little confused about this. The rwb->wb_max can't be 0 if the variable > 'limit' does not equal to 0. So the if (!rwb->wb_max) branch maybe does not > make sense. You are right, it doesn't make a lot of sense. I think it suffers from code shuffling. How about the attached? The important part is that we wake up waiters, if wbt got disabled while we had tracked IO in flight. diff --git a/lib/wbt.c b/lib/wbt.c index 650da911f24f..a6b80c135510 100644 --- a/lib/wbt.c +++ b/lib/wbt.c @@ -98,18 +98,23 @@ void __wbt_done(struct rq_wb *rwb) else limit = rwb->wb_normal; + inflight = atomic_dec_return(&rwb->inflight); + /* - * Don't wake anyone up if we are above the normal limit. If - * throttling got disabled (limit == 0) with waiters, ensure - * that we wake them up. + * wbt got disabled with IO in flight. Wake up any potential + * waiters, we don't have to do more than that. */ - inflight = atomic_dec_return(&rwb->inflight); - if (limit && inflight >= limit) { - if (!rwb->wb_max) - wake_up_all(&rwb->wait); + if (!rwb_enabled(rwb)) { + wake_up_all(&rwb->wait); return; } + /* + * Don't wake anyone up if we are above the normal limit. + */ + if (inflight >= limit) + return; + if (waitqueue_active(&rwb->wait)) { int diff = limit - inflight;