Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752038AbbKIJl5 (ORCPT ); Mon, 9 Nov 2015 04:41:57 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:44793 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751185AbbKIJlz (ORCPT ); Mon, 9 Nov 2015 04:41:55 -0500 Date: Mon, 9 Nov 2015 10:41:50 +0100 From: Peter Zijlstra To: Rusty Russell Cc: linux-kernel@vger.kernel.org, Josh Poimboeuf Subject: Re: [PATCH 3/4] module: use a structure to encapsulate layout. Message-ID: <20151109094150.GF17308@twins.programming.kicks-ass.net> References: <1447043037-10833-1-git-send-email-rusty@rustcorp.com.au> <1447043037-10833-4-git-send-email-rusty@rustcorp.com.au> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1447043037-10833-4-git-send-email-rusty@rustcorp.com.au> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3863 Lines: 123 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) -- 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/