lkml.org 
[lkml]   [2010]   [Oct]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 16/17] fs: Convert nr_inodes to a per-cpu counter
On Thu, Sep 30, 2010 at 04:10:39PM +1000, Dave Chinner wrote:
> On Wed, Sep 29, 2010 at 09:53:22PM -0700, Andrew Morton wrote:
> > On Wed, 29 Sep 2010 22:18:48 +1000 Dave Chinner <david@fromorbit.com> wrote:
> >
> > > From: Eric Dumazet <dada1@cosmosbay.com>
> > >
> > > The number of inodes allocated does not need to be tied to the
> > > addition or removal of an inode to/from a list. If we are not tied
> > > to a list lock, we could update the counters when inodes are
> > > initialised or destroyed, but to do that we need to convert the
> > > counters to be per-cpu (i.e. independent of a lock). This means that
> > > we have the freedom to change the list/locking implementation
> > > without needing to care about the counters.
> > >
> > >
> > > ...
> > >
> > > +int get_nr_inodes(void)
> > > +{
> > > + int i;
> > > + int sum = 0;
> > > + for_each_possible_cpu(i)
> > > + sum += per_cpu(nr_inodes, i);
> > > + return sum < 0 ? 0 : sum;
> > > +}
> >
> > This reimplements percpu_counter_sum_positive(), rather poorly

Why is it poorly?


> > If one never intends to use the approximate percpu_counter_read() then
> > one could initialise the counter with a really large batch value, for a
> > very small performance gain.

I did that to start with, and I was just looking to shave off cycles
and icache size. this_cpu_inc on x86 on a local variable is really
tiny and fast. percpu_counter does a function call which is large
and clobbers memory and registers, several branches, several loads and
stores, etc.

When it is a simple dumb statistics counter but with a critical
fastpath, this_cpu_inc just seems to be so much better.


> > > +int get_nr_inodes_unused(void)
> > > +{
> > > + return inodes_stat.nr_unused;
> > > +}
> > >
> > > ...
> > >
> > > @@ -407,6 +407,8 @@ extern struct files_stat_struct files_stat;
> > > extern int get_max_files(void);
> > > extern int sysctl_nr_open;
> > > extern struct inodes_stat_t inodes_stat;
> > > +extern int get_nr_inodes(void);
> > > +extern int get_nr_inodes_unused(void);
> >
> > These are pretty cruddy names. Unfotunately we don't really have a vfs
> > or "inode" subsystem name to prefix them with.

Any ideas? inodes_stat_nr_unused()?



\
 
 \ /
  Last update: 2010-10-16 09:59    [from the cache]
©2003-2011 Jasper Spaans