lkml.org 
[lkml]   [2011]   [Dec]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH] module: struct module_ref should contains long fields
    From
    Date
    Le vendredi 16 décembre 2011 à 08:05 -0800, Tejun Heo a écrit :
    > Hello,
    >
    > On Fri, Dec 16, 2011 at 8:01 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
    > > Its percpu data, there is no need to waste a full cache line per cpu for
    > > this.
    >
    > Hmmm.... okay. I just don't like seeing arbitrary alignment there.
    > Can you please add some comment explaining why the unusual alignment
    > is used there?

    Yep, thanks for the review !

    [PATCH v2] module: struct module_ref should contains long fields

    module_ref contains two "unsigned int" fields.

    Thats now too small, since some machines can open more than 2^32 files.

    Check commit 518de9b39e8 (fs: allow for more than 2^31 files) for
    reference.

    We can add an aligned(2 * sizeof(unsigned long)) attribute to force
    alloc_percpu() allocating module_ref areas in single cache lines.

    Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
    CC: Rusty Russell <rusty@rustcorp.com.au>
    CC: Tejun Heo <tj@kernel.org>
    CC: Robin Holt <holt@sgi.com>
    CC: David Miller <davem@davemloft.net>
    ---
    V2: added kerneldoc to struct module_ref

    include/linux/module.h | 21 ++++++++++++++++-----
    kernel/debug/kdb/kdb_main.c | 2 +-
    kernel/module.c | 8 ++++----
    3 files changed, 21 insertions(+), 10 deletions(-)

    diff --git a/include/linux/module.h b/include/linux/module.h
    index 3cb7839..4598bf0 100644
    --- a/include/linux/module.h
    +++ b/include/linux/module.h
    @@ -205,6 +205,20 @@ enum module_state
    MODULE_STATE_GOING,
    };

    +/**
    + * struct module_ref - per cpu module reference counts
    + * @incs: number of module get on this cpu
    + * @decs: number of module put on this cpu
    + *
    + * We force an alignment on 8 or 16 bytes, so that alloc_percpu()
    + * put @incs/@decs in same cache line, with no extra memory cost,
    + * since alloc_percpu() is fine grained.
    + */
    +struct module_ref {
    + unsigned long incs;
    + unsigned long decs;
    +} __attribute((aligned(2 * sizeof(unsigned long))));
    +
    struct module
    {
    enum module_state state;
    @@ -347,10 +361,7 @@ struct module
    /* Destruction function. */
    void (*exit)(void);

    - struct module_ref {
    - unsigned int incs;
    - unsigned int decs;
    - } __percpu *refptr;
    + struct module_ref __percpu *refptr;
    #endif

    #ifdef CONFIG_CONSTRUCTORS
    @@ -434,7 +445,7 @@ extern void __module_put_and_exit(struct module *mod, long code)
    #define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code);

    #ifdef CONFIG_MODULE_UNLOAD
    -unsigned int module_refcount(struct module *mod);
    +unsigned long module_refcount(struct module *mod);
    void __symbol_put(const char *symbol);
    #define symbol_put(x) __symbol_put(MODULE_SYMBOL_PREFIX #x)
    void symbol_put_addr(void *addr);
    diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
    index 63786e7..e2ae734 100644
    --- a/kernel/debug/kdb/kdb_main.c
    +++ b/kernel/debug/kdb/kdb_main.c
    @@ -1982,7 +1982,7 @@ static int kdb_lsmod(int argc, const char **argv)
    kdb_printf("%-20s%8u 0x%p ", mod->name,
    mod->core_size, (void *)mod);
    #ifdef CONFIG_MODULE_UNLOAD
    - kdb_printf("%4d ", module_refcount(mod));
    + kdb_printf("%4ld ", module_refcount(mod));
    #endif
    if (mod->state == MODULE_STATE_GOING)
    kdb_printf(" (Unloading)");
    diff --git a/kernel/module.c b/kernel/module.c
    index 178333c..ef05f7e 100644
    --- a/kernel/module.c
    +++ b/kernel/module.c
    @@ -726,9 +726,9 @@ static int try_stop_module(struct module *mod, int flags, int *forced)
    }
    }

    -unsigned int module_refcount(struct module *mod)
    +unsigned long module_refcount(struct module *mod)
    {
    - unsigned int incs = 0, decs = 0;
    + unsigned long incs = 0, decs = 0;
    int cpu;

    for_each_possible_cpu(cpu)
    @@ -854,7 +854,7 @@ static inline void print_unload_info(struct seq_file *m, struct module *mod)
    struct module_use *use;
    int printed_something = 0;

    - seq_printf(m, " %u ", module_refcount(mod));
    + seq_printf(m, " %lu ", module_refcount(mod));

    /* Always include a trailing , so userspace can differentiate
    between this and the old multi-field proc format. */
    @@ -904,7 +904,7 @@ EXPORT_SYMBOL_GPL(symbol_put_addr);
    static ssize_t show_refcnt(struct module_attribute *mattr,
    struct module_kobject *mk, char *buffer)
    {
    - return sprintf(buffer, "%u\n", module_refcount(mk->mod));
    + return sprintf(buffer, "%lu\n", module_refcount(mk->mod));
    }

    static struct module_attribute refcnt = {

    --
    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: 2011-12-16 17:39    [W:0.033 / U:32.104 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site