Re: [PATCH] fs: use approximate counter values for inodes and dentries. (was Re: [patch] fs: use fast counters for vfs caches)

[Nick Piggin <npiggin@kernel.dk>]

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_counter
> > code over the superior per cpu data that I was using.
> 
> 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).

 
> 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, unless you
> > have numbers)
> 
> 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.

 
> > > It certainly wasn't measurable on my
> > > 16p machine, and nobody who reviewed it at the time (ѕeveral people)
> > > picked it up. So thanks for reviewing it - the simple fix is below.
> > >
> > > Cheers,
> > >
> > > Dave.
> > > --
> > > Dave Chinner
> > > david@fromorbit.com
> > >
> > > fs: Use approximate values for number of inodes and dentries
> > >
> > > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Nack. Can you please address my points and actually explain why this
> > is better than my proposed approach please?
> 
> 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