Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753348AbcD0PWU (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 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> Content-Type: multipart/mixed; boundary="------------010104070102020004060802" 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-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2429 Lines: 86 --------------010104070102020004060802 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit 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. -- Jens Axboe --------------010104070102020004060802 Content-Type: text/x-patch; name="wbt-disable-wakeup.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="wbt-disable-wakeup.patch" 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; --------------010104070102020004060802--