Messages in this thread | | | Date | Wed, 11 Sep 2013 11:21:49 -0400 | From | Konrad Rzeszutek Wilk <> | Subject | Re: Regression :-) Re: [GIT PULL RESEND] x86/jumpmplabel changes for v3.12-rc1 |
| |
On Wed, Sep 11, 2013 at 10:56:33AM -0400, Steven Rostedt wrote: > On Wed, 11 Sep 2013 10:25:45 -0400 > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > > > It seems to imply line 53 is the originating bug, so that would be: > > > > > > 47 if (type == JUMP_LABEL_ENABLE) { > > > 48 /* > > > 49 * We are enabling this jump label. If it is not a nop > > > 50 * then something must have gone wrong. > > > 51 */ > > > 52 if (unlikely(memcmp((void *)entry->code, ideal_nop, 5) != 0)) > > > 53 bug_at((void *)entry->code, __LINE__); > > > > > > But it is a NOP isn't it? The code is > > > > > > Unexpected op at trace_clock_global+0x6b/0x120 [ffffffff8113a21b] (0f 1f 44 00 00) 53 > > > > > > Perhaps the ideal_nop has not been set yet? > > > > > > > And this looks to fix it for me. > > I'm trying to understand how this will fix it for you. Are you sure you > removed 'xen_nopvspin'?
Yes. > > If you are calling static_key_slow_inc() before jump_label_init(), then > it should still fail. The static_key_slow_inc() eventually calls > arch_jump_label_transform(), which calls __jump_label_transform() with > init == 0.
Perhaps I am misreading the code, but I believe init is set to one. That is due to us calling:
arch_jump_label_transform (.., JUMP_LABEL_ENABLE)
which calls __jump_label_transform(.., 1) ?
Perhaps the 'init' and 'enable' parameters have different meanings?
> > The below code looks to me that it would still compare the contents > with the ideal_nop, which hasn't been set yet.
In the !init case - sure.
In the init case - just with default_nop.
> > -- Steve > > > > > diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c > > index ee11b7d..d688348 100644 > > --- a/arch/x86/kernel/jump_label.c > > +++ b/arch/x86/kernel/jump_label.c > > @@ -44,13 +44,20 @@ 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)) > > + bug_at((void *)entry->code, __LINE__); > > + } > > > > code.jump = 0xe9; > > code.offset = entry->target - > > @@ -62,11 +69,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@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
| |