Messages in this thread | | | Date | Mon, 9 Nov 2015 10:41:50 +0100 | From | Peter Zijlstra <> | Subject | Re: [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>
| |