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    [W:0.025 / U:29.444 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site