Date: Mon, 13 Apr 2015 18:56:36 +0200
From: Ingo Molnar <mingo@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: rusty@rustcorp.com.au, mathieu.desnoyers@efficios.com, oleg@redhat.com,
        paulmck@linux.vnet.ibm.com, torvalds@linux-foundation.org,
        linux-kernel@vger.kernel.org, andi@firstfloor.org, rostedt@goodmis.org,
        tglx@linutronix.de, laijs@cn.fujitsu.com, linux@horizon.com
Subject: Re: [PATCH v5 10/10] module: Rework module_addr_{min,max}
Message-ID: <20150413165636.GH6040@gmail.com>
References: <20150413141126.756350256@infradead.org>
 <20150413141213.800009335@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150413141213.800009335@infradead.org>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1776
Lines: 53


* Peter Zijlstra <peterz@infradead.org> wrote:

> __module_address() does an initial bound check before doing the 
> {list/tree} iteration to find the actual module. The bound variables 
> are nowhere near the mod_tree cacheline, in fact they're nowhere 
> near one another.
> 
> module_addr_min lives in .data while module_addr_max lives in .bss 
> (smarty pants GCC thinks the explicit 0 assignment is a mistake).
> 
> Rectify this by moving the two variables into a structure together 
> with the latch_tree_root to guarantee they all share the same 
> cacheline and avoid hitting two extra cachelines for the lookup.
> 
> While reworking the bounds code, move the bound update from 
> allocation to insertion time, this avoids updating the bounds for a 
> few error paths.

> +static struct mod_tree_root {
> +	struct latch_tree_root root;
> +	unsigned long addr_min;
> +	unsigned long addr_max;
> +} mod_tree __cacheline_aligned = {
> +	.addr_min = -1UL,
> +};
> +
> +#define module_addr_min mod_tree.addr_min
> +#define module_addr_max mod_tree.addr_max

>  #else /* !(PERF_EVENTS || TRACING) */
>  
> +/*
> + * Bounds of module allocation, for speeding up __module_address.
> + * Protected by module_mutex.
> + */
> +static unsigned long module_addr_min = -1UL, module_addr_max = 0;

I suspect the same .data vs. .bss problem affects the #else branch as 
well?

If so then it would make sense IMHO to put the structure definition 
into generic code so that both variants benefit from the shared 
cacheline?

Thanks,

	Ingo
--
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/