Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757375AbZLITwD (ORCPT ); Wed, 9 Dec 2009 14:52:03 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1757342AbZLITv5 (ORCPT ); Wed, 9 Dec 2009 14:51:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:31765 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757341AbZLITv4 (ORCPT ); Wed, 9 Dec 2009 14:51:56 -0500 From: Jeff Moyer To: Corrado Zoccolo Cc: Jens Axboe , "Linux-Kernel" , Vivek Goyal Subject: Re: [PATCH] cfq-iosched: reduce write depth only if sync was delayed References: <4b1ffe39.0f1abc0a.19c7.05d2@mx.google.com> X-PGP-KeyID: 1F78E1B4 X-PGP-CertKey: F6FE 280D 8293 F72C 65FD 5A58 1FF8 A7CA 1F78 E1B4 X-PCLoadLetter: What the f**k does that mean? Date: Wed, 09 Dec 2009 14:51:51 -0500 In-Reply-To: <4b1ffe39.0f1abc0a.19c7.05d2@mx.google.com> (Corrado Zoccolo's message of "Wed, 9 Dec 2009 20:45:16 +0100") Message-ID: User-Agent: Gnus/5.110011 (No Gnus v0.11) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2487 Lines: 62 Corrado Zoccolo writes: > On Wed, Dec 9, 2009 at 8:05 PM, Jens Axboe wrote: >> On Wed, Dec 09 2009, Jeff Moyer wrote: >>> OK. Can we put a comment in there and change the initialization to >>> cfq_slice_sync * 10? >> >> Agree, that would be MUCH easier to understand. >> > Sure, we can put a comment there, but I don't like hardcoding a > constant that depends on how the formula is computed (what if the > formula is changed, and it doesn't depend on cfq_slice_sync any more, > or if cfq_slice_sync changes dynamically?). Then presumably you'd change the initialization of that variable. > When I wrote it, what I really meant was exactly what you read in the > C code (assume the last delayed sync happened 1 second ago). Then, the > effect would be to start with a queue depth of 10 with the current > formula, but even if we change the formula, 1 second is still > meaningful (while 10 *cfq_slice_sync, that has the same value, becomes > misleading). So my proposed fix is just: Well, given your initial explanation, my suggestion made sense to me. Given this new explanation, I'm fine with the change below. Thanks for clarifying. Acked-by: Jeff Moyer > From f06cd83b45b3a7ee13ae7322197b610085dc70dd Mon Sep 17 00:00:00 2001 > From: Corrado Zoccolo > Date: Wed, 9 Dec 2009 20:40:16 +0100 > Subject: [PATCH] cfq-iosched: commenting non-obvious initialization > > Added a comment to explain the initialization of last_delayed_sync. > > Signed-off-by: Corrado Zoccolo > --- > block/cfq-iosched.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c > index 98b15b9..69ecee7 100644 > --- a/block/cfq-iosched.c > +++ b/block/cfq-iosched.c > @@ -3759,6 +3759,10 @@ static void *cfq_init_queue(struct request_queue *q) > cfqd->cfq_latency = 1; > cfqd->cfq_group_isolation = 0; > cfqd->hw_tag = -1; > + /* > + * we optimistically start assuming sync ops weren't delayed in last > + * second, in order to have larger depth for async operations. > + */ > cfqd->last_delayed_sync = jiffies - HZ; > INIT_RCU_HEAD(&cfqd->rcu); > return cfqd; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/