From: Eric Sandeen Subject: Re: [PATCH] ext4: use percpu counter for extent cache count Date: Mon, 04 Mar 2013 10:11:09 -0600 Message-ID: <5134C79D.6020901@redhat.com> References: <20130227185625.GA224@x4> <20130227191923.GA1121@redhat.com> <20130227192907.GB14253@thunk.org> <20130227201217.GD14253@thunk.org> <20130301033005.GA7081@redhat.com> <20130301040039.GA4452@thunk.org> <20130301050029.GB4452@thunk.org> <5130DA71.4040808@redhat.com> <20130301180035.GB17920@thunk.org> <5130ED3A.30003@redhat.com> <20130302152644.GA22941@thunk.org> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Dave Jones , "gnehzuil.liu" , Zheng Liu , "linux-ext4@vger.kernel.org" To: "Theodore Ts'o" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:34476 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932133Ab3CDQL1 (ORCPT ); Mon, 4 Mar 2013 11:11:27 -0500 In-Reply-To: <20130302152644.GA22941@thunk.org> Sender: linux-ext4-owner@vger.kernel.org List-ID: On 3/2/13 9:26 AM, Theodore Ts'o wrote: > On Fri, Mar 01, 2013 at 12:02:34PM -0600, Eric Sandeen wrote: >> >> I was mostly keying off what quota felt was best, I guess. >> I'm not wedded to either approach, it was just a thought. >> So you can take it or leave it. :) > > I'll take it, but for the record, there's a reason why I label patches > with "I haven't tested this yet" disclaimers. > > For future reference, calling percpu_counter_init() and > percpu_counter_destroy() is not optional --- without them, in your > original patch, the kernel will crash them the first time an ext4 file > system mount is attempted. > > No worries though, no harm done --- but it's better if people don't > rely on me to catch bugs before they go into the tree. :-) Oh crud, I'm really sorry. I originally had the patch embedded in a reply, and it did say "not tested" and was more of a "what do you think about this" - then I elevated it to its own mail in the thread and I lost the disclaimer. Entirely my fault. :( That's two stupid-patch-demerits for me from Friday. Only one one ext4. ;) -Eric > - Ted > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 373d46c..1ae5860 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -783,6 +783,7 @@ static void ext4_put_super(struct super_block *sb) > percpu_counter_destroy(&sbi->s_freeinodes_counter); > percpu_counter_destroy(&sbi->s_dirs_counter); > percpu_counter_destroy(&sbi->s_dirtyclusters_counter); > + percpu_counter_destroy(&sbi->s_extent_cache_cnt); > brelse(sbi->s_sbh); > #ifdef CONFIG_QUOTA > for (i = 0; i < MAXQUOTAS; i++) > @@ -3688,6 +3689,9 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) > if (!err) { > err = percpu_counter_init(&sbi->s_dirtyclusters_counter, 0); > } > + if (!err) { > + err = percpu_counter_init(&sbi->s_extent_cache_cnt, 0); > + } > if (err) { > ext4_msg(sb, KERN_ERR, "insufficient memory"); > goto failed_mount3; > @@ -3993,6 +3997,7 @@ failed_mount3: > percpu_counter_destroy(&sbi->s_freeinodes_counter); > percpu_counter_destroy(&sbi->s_dirs_counter); > percpu_counter_destroy(&sbi->s_dirtyclusters_counter); > + percpu_counter_destroy(&sbi->s_extent_cache_cnt); > if (sbi->s_mmp_tsk) > kthread_stop(sbi->s_mmp_tsk); > failed_mount2: >