From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935058Ab3JPOOa (ORCPT ); Wed, 16 Oct 2013 10:14:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1491 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934648Ab3JPOO2 (ORCPT ); Wed, 16 Oct 2013 10:14:28 -0400 Date: Wed, 16 Oct 2013 10:14:06 -0400 From: Vivek Goyal To: Hong zhi guo Cc: Jens Axboe , cgroups@vger.kernel.org, Tejun Heo , linux-kernel@vger.kernel.org, Hong Zhiguo Subject: Re: [PATCH v2] blk-throttle: simplify logic by token bucket algorithm Message-ID: <20131016141405.GE17611@redhat.com> References: <1381574794-7639-1-git-send-email-zhiguohong@tencent.com> <1381741757-20888-1-git-send-email-zhiguohong@tencent.com> <20131015173252.GM31215@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 16, 2013 at 02:09:40PM +0800, Hong zhi guo wrote: > Hi, Vivek, > > Thanks for your careful review. I'll rename t_c to last_dispatch, it's > much better. > > For the big burst issue, I've different opinion. Let's discuss it. > > Any time a big IO means a big burst. Even if it's throttled at first > time, queued in > the service_queue, and then waited for a while, when it's delivered > out, it IS still > a big burst. So throttling the bio for another while makes no sense. If a malicious application is creating a big BIO and sending it out, then effective IO rate as seen by application will be much higher than throttling limit. So yes, a burst is anyway going to happen when big IO is dispatched to disk, but the question is when should that burst be allowed. What's the effective IO rate application should see. > > If a group has been idle for 5 minutes, then it owns the credit to > deliver a big IO > (within 300 * bps bytes). And the extra credit will be cut off after > the delivery. I think there are couple of issues here. - First of all, if you think that a group is entitiled for tokens even when it is not doing IO, then why are you truncating the tokens after dispatch of a BIO. - Second in general it does not seem right that a group is entitiled to tokens even when no IO is happening or group is not backlogged. That would mean a group will not do IO for 10 hours and then be entitiled to those tokens suddenly after 10 hours with a huge burst. So I think you also agree that a group should not be entitiled to tokens when group is not backlogged and that's why you seem to be truncating extra tokens after dispatch of a BIO. If that's the case, then even for first BIO, ideally a group should not be given tokens for idle time. Just think that an application has a huge BIO, say size 2MB. And group has limit of say 8KB per second. Now if group has been idling long enough, this BIO will be dispatched immediately. And effective rate a group will be is much higher than 8KB/s. Which is not right, IMO. If you argue that token entitilement for idle groups is not right and doing it for first BIO in a batch is exception for simplicity reasons, that still might be fine. But right now that does not seem to be the case. Thanks Vivek