lkml.org 
[lkml]   [2006]   [Feb]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 2/2] fix file counting
    Dipankar Sarma <dipankar@in.ibm.com> wrote:
    >
    > I have benchmarked this on an x86_64 NUMA system and see no
    > significant performance difference on kernbench. Tested on
    > both x86_64 and powerpc.
    >
    > The way we do file struct accounting is not very suitable for batched
    > freeing. For scalability reasons, file accounting was constructor/destructor
    > based. This meant that nr_files was decremented only when
    > the object was removed from the slab cache. This is
    > susceptible to slab fragmentation. With RCU based file structure,
    > consequent batched freeing and a test program like Serge's,
    > we just speed this up and end up with a very fragmented slab -
    >
    > llm22:~ # cat /proc/sys/fs/file-nr
    > 587730 0 758844
    >
    > At the same time, I see only a 2000+ objects in filp cache.
    > The following patch I fixes this problem.
    >
    > This patch changes the file counting by removing the filp_count_lock.
    > Instead we use a separate percpu counter, nr_files, for now and all
    > accesses to it are through get_nr_files() api. In the sysctl
    > handler for nr_files, we populate files_stat.nr_files before returning
    > to user.
    >
    > Counting files as an when they are created and destroyed (as opposed
    > to inside slab) allows us to correctly count open files with RCU.

    Fair enough.

    What do you think of these changes?




    From: Andrew Morton <akpm@osdl.org>

    - Nuke the blank line between "}" and EXPORT_SYMBOL(). That's never seemed
    pointful to me.

    - Make the get_max_files export use _GPL - only unix.ko uses it.

    - Use `-1' in the arg to percpu_counter_mod() rather than `-1L'. The
    compiler will dtrt and we shouldn't be peering inside percpu_counter
    internals here anyway.

    - Scrub that - use percpu_counter_dec() and percpu_counter_inc().

    - percpu_counters can be inaccurate on big SMP. Before we actually fail a
    get_empty_filp() attempt, use the (new in -mm) expensive
    percpu_counter_sum() to check whether we're really over the limit.

    - Make get_nr_files() static. Which is just as well - any callers might
    want the percpu_counter_sum() treatment. In which case we'd be better off
    exporting some

    bool are_files_over_limit(how_many_i_want);

    API.

    Cc: Dipankar Sarma <dipankar@in.ibm.com>
    Cc: "Paul E. McKenney" <paulmck@us.ibm.com>
    Signed-off-by: Andrew Morton <akpm@osdl.org>
    ---

    fs/file_table.c | 21 +++++++++++++--------
    include/linux/fs.h | 1 -
    2 files changed, 13 insertions(+), 9 deletions(-)

    diff -puN fs/file_table.c~fix-file-counting-fixes fs/file_table.c
    --- devel/fs/file_table.c~fix-file-counting-fixes 2006-02-18 01:02:43.000000000 -0800
    +++ devel-akpm/fs/file_table.c 2006-02-18 01:02:43.000000000 -0800
    @@ -22,6 +22,7 @@
    #include <linux/fsnotify.h>
    #include <linux/sysctl.h>
    #include <linux/percpu_counter.h>
    +
    #include <asm/atomic.h>

    /* sysctl tunables... */
    @@ -42,14 +43,14 @@ static inline void file_free_rcu(struct

    static inline void file_free(struct file *f)
    {
    - percpu_counter_mod(&nr_files, -1L);
    + percpu_counter_dec(&nr_files);
    call_rcu(&f->f_u.fu_rcuhead, file_free_rcu);
    }

    /*
    * Return the total number of open files in the system
    */
    -int get_nr_files(void)
    +static int get_nr_files(void)
    {
    return percpu_counter_read_positive(&nr_files);
    }
    @@ -61,8 +62,7 @@ int get_max_files(void)
    {
    return files_stat.max_files;
    }
    -
    -EXPORT_SYMBOL(get_max_files);
    +EXPORT_SYMBOL_GPL(get_max_files);

    /*
    * Handle nr_files sysctl
    @@ -95,15 +95,20 @@ struct file *get_empty_filp(void)
    /*
    * Privileged users can go above max_files
    */
    - if (get_nr_files() >= files_stat.max_files &&
    - !capable(CAP_SYS_ADMIN))
    - goto over;
    + if (get_nr_files() >= files_stat.max_files && !capable(CAP_SYS_ADMIN)) {
    + /*
    + * percpu_counters are inaccurate. Do an expensive check before
    + * we go and fail.
    + */
    + if (percpu_counter_sum(&nr_files) >= files_stat.max_files)
    + goto over;
    + }

    f = kmem_cache_alloc(filp_cachep, GFP_KERNEL);
    if (f == NULL)
    goto fail;

    - percpu_counter_mod(&nr_files, 1L);
    + percpu_counter_inc(&nr_files);
    memset(f, 0, sizeof(*f));
    if (security_file_alloc(f))
    goto fail_sec;
    diff -puN include/linux/fs.h~fix-file-counting-fixes include/linux/fs.h
    --- devel/include/linux/fs.h~fix-file-counting-fixes 2006-02-18 01:02:43.000000000 -0800
    +++ devel-akpm/include/linux/fs.h 2006-02-18 01:02:43.000000000 -0800
    @@ -35,7 +35,6 @@ struct files_stat_struct {
    int max_files; /* tunable */
    };
    extern struct files_stat_struct files_stat;
    -extern int get_nr_files(void);
    extern int get_max_files(void);

    struct inodes_stat_t {
    _
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-02-18 10:08    [W:0.079 / U:30.276 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site