From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [PATCH] fs: use approximate counter values for inodes and dentries. (was Re: [patch] fs: use fast counters for vfs caches) Date: Fri, 10 Dec 2010 13:23:58 +1100 Message-ID: <20101210022358.GB3331@amd> References: <20101129105733.GA3241@amd> <20101209054343.GA8259@dastard> <20101209061644.GA3667@amd> <20101209074503.GD8259@dastard> <20101209233028.GA9925@dastard> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Nick Piggin , Nick Piggin , linux-fsdevel@vger.kernel.org, Linus Torvalds , Al Viro , Christoph Hellwig To: Dave Chinner Return-path: Received: from ipmail07.adl2.internode.on.net ([150.101.137.131]:64028 "EHLO ipmail07.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751363Ab0LJCYD (ORCPT ); Thu, 9 Dec 2010 21:24:03 -0500 Content-Disposition: inline In-Reply-To: <20101209233028.GA9925@dastard> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Dec 10, 2010 at 10:30:28AM +1100, Dave Chinner wrote: > On Thu, Dec 09, 2010 at 11:24:38PM +1100, Nick Piggin wrote: > > No. I was referring to the decision to use the heavyweight percpu_c= ounter > > code over the superior per cpu data that I was using. >=20 > Your "superior" solution is only superior when you don't have to sum > the counters regularly. I was talking about using per cpu variable only for the total counts. The unused counts would be per-lru (ie. a global variable in this case). =20 > I'll repeat what Andrew Morton said early one when your per-cpu > counter approach was first discussed: If you think the generic > percpu counters are too heavyweight, then _fix the generic counters_ > rather than hack around them. That way everyone who uses the generic > infrastructure benefits and it reduces the desire for every subsystem > to roll their own specialised percpu counters... So why was the percpu_counter patch merged without addressing *my* concern that it is too heavyweight? Hmm? > > Also, the unrelated change to make nr_unused into per-cpu was not > > right, and I will revert that back to a global variable. (again, un= less you > > have numbers) >=20 > What "nr_unused" variable? nr_dentrys_unused, nr_inodes_unused or > some other variable? And, apart from the overhead, why is it wrong - > does it give incorrect values? It's wrong because it is tied completely to lru operation and can't be at all scalable anyway. I said that in this thread already, there is no point adding overhead of per cpu counter for operations that are done under a lock anyway. =20 > > > It certainly wasn't measurable on my > > > 16p machine, and nobody who reviewed it at the time (=D1=95everal= people) > > > picked it up. So thanks for reviewing it - the simple fix is belo= w. > > > > > > Cheers, > > > > > > Dave. > > > -- > > > Dave Chinner > > > david@fromorbit.com > > > > > > fs: Use approximate values for number of inodes and dentries > > > > > > From: Dave Chinner > >=20 > > Nack. Can you please address my points and actually explain why thi= s > > is better than my proposed approach please? >=20 > FFS. What bit of "need to sum the counters thousands of times a > second" don't you understand? The part where reclaim only sums the nr_unused counter, which I said should not be per cpu. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html