lkml.org 
[lkml]   [2015]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/4] module: use a structure to encapsulate layout.
On Mon, Nov 09, 2015 at 02:53:56PM +1030, Rusty Russell wrote:

> diff --git a/include/linux/module.h b/include/linux/module.h
> index 3a19c79918e0..6e68e8cf4d0d 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -302,6 +302,28 @@ struct mod_tree_node {
> struct latch_tree_node node;
> };
>
> +struct module_layout {
> + /* The actual code + data. */
> + void *base;
> + /* Total size. */
> + unsigned int size;
> + /* The size of the executable code. */
> + unsigned int text_size;
> + /* Size of RO section of the module (text+rodata) */
> + unsigned int ro_size;

There's a 4 byte hole here, but I suppose that's OK, this arrangement
does simplify things.

> +
> +#ifdef CONFIG_MODULES_TREE_LOOKUP
> + struct mod_tree_node mtn;
> +#endif
> +};
> +
> +#ifdef CONFIG_MODULES_TREE_LOOKUP
> +/* Only touch one cacheline for common rbtree-for-core-layout case. */
> +#define __module_layout_align ____cacheline_aligned
> +#else
> +#define __module_layout_align
> +#endif
> +
> struct module {
> enum module_state state;
>

> diff --git a/kernel/module.c b/kernel/module.c
> index 14b224967e7b..a0a3d6d9d5e8 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -108,13 +108,6 @@ static LIST_HEAD(modules);
> * Use a latched RB-tree for __module_address(); this allows us to use
> * RCU-sched lookups of the address from any context.
> *
> - * Because modules have two address ranges: init and core, we need two
> - * latch_tree_nodes entries. Therefore we need the back-pointer from
> - * mod_tree_node.

We still have the back-pointers, so removing all of that seems a little
excessive.

> - *
> - * Because init ranges are short lived we mark them unlikely and have placed
> - * them outside the critical cacheline in struct module.

This information also isn't preserved.

> * This is conditional on PERF_EVENTS || TRACING because those can really hit
> * __module_address() hard by doing a lot of stack unwinding; potentially from
> * NMI context.
> @@ -122,24 +115,16 @@ static LIST_HEAD(modules);
>
> static __always_inline unsigned long __mod_tree_val(struct latch_tree_node *n)
> {
> - struct mod_tree_node *mtn = container_of(n, struct mod_tree_node, node);
> - struct module *mod = mtn->mod;
> + struct module_layout *layout = container_of(n, struct module_layout, mtn.node);
>
> - if (unlikely(mtn == &mod->mtn_init))
> - return (unsigned long)mod->module_init;
> -
> - return (unsigned long)mod->module_core;
> + return (unsigned long)layout->base;
> }
>
> static __always_inline unsigned long __mod_tree_size(struct latch_tree_node *n)
> {
> - struct mod_tree_node *mtn = container_of(n, struct mod_tree_node, node);
> - struct module *mod = mtn->mod;
> -
> - if (unlikely(mtn == &mod->mtn_init))
> - return (unsigned long)mod->init_size;
> + struct module_layout *layout = container_of(n, struct module_layout, mtn.node);
>
> - return (unsigned long)mod->core_size;
> + return (unsigned long)layout->size;
> }

Nice!

> @@ -197,23 +182,23 @@ static void __mod_tree_remove(struct mod_tree_node *node)
> */
> static void mod_tree_insert(struct module *mod)
> {
> - mod->mtn_core.mod = mod;
> - mod->mtn_init.mod = mod;
> + mod->core_layout.mtn.mod = mod;
> + mod->init_layout.mtn.mod = mod;

^ back-pointers :-)

> - __mod_tree_insert(&mod->mtn_core);
> - if (mod->init_size)
> - __mod_tree_insert(&mod->mtn_init);
> + __mod_tree_insert(&mod->core_layout.mtn);
> + if (mod->init_layout.size)
> + __mod_tree_insert(&mod->init_layout.mtn);
> }

Aside from these minor nits,

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>




\
 
 \ /
  Last update: 2015-11-09 11:01    [W:0.170 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site