Date:	Wed, 11 Sep 2013 13:05:07 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Cc:	"H. Peter Anvin" <hpa@...ux.intel.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...e.hu>,
	Jason Baron <jbaron@...mai.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	boris.ostrovsky@...cle.com, david.vrabel@...rix.com
Subject: Re: Regression :-) Re: [GIT PULL RESEND] x86/jumpmplabel changes
 for v3.12-rc1


[ Fixed Jason Baron's email so that he can join the conversation ]

On Wed, 11 Sep 2013 12:17:45 -0400
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com> wrote:

> On Wed, Sep 11, 2013 at 11:47:08AM -0400, Steven Rostedt wrote:

> [    4.966101] Kernel command line: debug selinux=0 earlyprintk=xen console=hvc0 xencons=hvc0 loglevel=10 pci=resource_alignment=00:13.2 xen-pciback.hide=(08:07.0)(08:06.0)(00:12.0)(00:12.1)(00:12.2)(00:13.0)(00:13.1)(00:13.2)(00:14.5) xen-pciback.passthrough=0
> [    4.966892] op trace_clock_global+0x6b/0x120
> [    4.966895] CPU: 0 PID: 0 Comm: swapper Not tainted 3.11.0upstream-09031-ga22a0fd-dirty #1
> [    4.966897] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS 080014  07/18/2008
> [    4.966899]  ffffffff810542e0 ffffffff81c01e28 ffffffff816a0cf3 0000000000000001
> [    4.966903]  ffffffff81ca8598 ffffffff81c01e88 ffffffff81051e0a ffffffe8ffffffe8
> [    4.966905]  0000001800000000 ffffffff81162980 0000000000000018 ffffff0000441f0f
> [    4.966907] Call Trace:
> [    4.966912]  [<ffffffff810542e0>] ? poke_int3_handler+0x40/0x40
> [    4.966916]  [<ffffffff816a0cf3>] dump_stack+0x59/0x7b
> [    4.966920]  [<ffffffff81051e0a>] __jump_label_transform+0x18a/0x230
> [    4.966923]  [<ffffffff81162980>] ? fire_user_return_notifiers+0x70/0x70
> [    4.966926]  [<ffffffff81051f15>] arch_jump_label_transform_static+0x65/0x90
> [    4.966930]  [<ffffffff81cfbbfb>] jump_label_init+0x75/0xa3
> [    4.966932]  [<ffffffff81cd3e3c>] start_kernel+0x168/0x3ff
> [    4.966934]  [<ffffffff81cd3af2>] ? repair_env_string+0x5b/0x5b
> [    4.966938]  [<ffffffff81cd35f3>] x86_64_start_reservations+0x2a/0x2c
> [    4.966941]  [<ffffffff81cd833a>] xen_start_kernel+0x594/0x596
> [    4.967072] PID hash table entries: 4096 (order: 3, 32768 bytes)
> [    5.009945] software IO TLB [mem 0x3a400000-0x3e400000] (64MB) mapped at [ffff88003a400000-ffff88003e3fffff]
> [    5.013794] Memory: 868480K/1048572K available (6860K kernel code, 752K rwdata, 2140K rodata, 1708K init, 1876K bss, 180092K reserved)
> [    5.014212] Hierarchical RCU implementation.
> [    5.014214] 	RCU restricting CPUs from NR_CPUS=512 to nr_cpu_ids=4.
> [    5.014229] NR_IRQS:33024 nr_irqs:712 16
> [    5.014370] xen: sci override: global_irq=9 trigger=0 polarity=1
> 
> .... snip.
> 
> And here is the patch:
> 
> diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
> index ee11b7d..e3a41a0 100644
> --- a/arch/x86/kernel/jump_label.c
> +++ b/arch/x86/kernel/jump_label.c
> @@ -44,13 +44,31 @@ static void __jump_label_transform(struct jump_entry *entry,
>  	union jump_code_union code;
>  	const unsigned char *ideal_nop = ideal_nops[NOP_ATOMIC5];
>  
> +	if (init) {
> +		const unsigned char default_nop[] = { STATIC_KEY_INIT_NOP };
> +		if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
> +			bug_at((void *)entry->code, __LINE__);
> +	}
>  	if (type == JUMP_LABEL_ENABLE) {
>  		/*
>  		 * We are enabling this jump label. If it is not a nop
>  		 * then something must have gone wrong.
>  		 */
> -		if (unlikely(memcmp((void *)entry->code, ideal_nop, 5) != 0))
> -			bug_at((void *)entry->code, __LINE__);
> +		if (init) {
> +			if (unlikely(memcmp((void *)entry->code, ideal_nop, 5) != 0)) {
> +				static int log = 0;
> +
> +				if (log == 0) {
> +					pr_warning("op %pS\n", (void *)entry->code);
> +					dump_stack();

OK, I think I understand the problem, and this may or may not be a real
bug depending on what the jump label infrastructure expects.

Jason,

How safe is it to use static_key_slow_inc() before jump_label_init() is
called?

What happened here is that the xen code called by
smp_prepare_boot_cpu() checks boot parameters and may do a
static_key_slow_inc() if xen_nopvspin is not set. Which basically
enables a jump label. The issues is that because jump_labels have not
been initialized yet, it just ups the "enable" count and does not
modify anything because key->entries is still NULL.

When jump_label_init() is called, it sees that the branch is enabled
and then converts it to being enabled, but here's where the current
check fails. It does not expect a jump label to be already enabled when
it gets here.

Now, if it is fine to enable a jump label before jump_label_init() then
I will agree that this patch is the proper fix. But before I give my
Ack, I want to know if the jump label infrastructure was designed to
allow enabling of jump labels at boot up before jump_label_init() is
run.

-- Steve




> +				}
> +				log++;
> +			}
> +		}
> +		if (!init) {
> +			if (unlikely(memcmp((void *)entry->code, ideal_nop, 5) != 0))
> +				bug_at((void *)entry->code, __LINE__);
> +		}
>  
>  		code.jump = 0xe9;
>  		code.offset = entry->target -
> @@ -62,11 +80,7 @@ static void __jump_label_transform(struct jump_entry *entry,
>  		 * If this is the first initialization call, then we
>  		 * are converting the default nop to the ideal nop.
>  		 */
> -		if (init) {
> -			const unsigned char default_nop[] = { STATIC_KEY_INIT_NOP };
> -			if (unlikely(memcmp((void *)entry->code, default_nop, 5) != 0))
> -				bug_at((void *)entry->code, __LINE__);
> -		} else {
> +		if (!init) {
>  			code.jump = 0xe9;
>  			code.offset = entry->target -
>  				(entry->code + JUMP_LABEL_NOP_SIZE);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists