lkml.org 
[lkml]   [2014]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 13/15] sched: Use a static_key for sched_clock_stable
On Tue, Jan 21, 2014 at 05:28:37PM -0500, Sasha Levin wrote:
> On 12/12/2013 09:08 AM, Peter Zijlstra wrote:
>
> This patch seems to be causing an issue with booting a KVM guest. It seems that it
> causes the time to go random during early boot process:
>
> [ 0.000000] Initmem setup node 30 [mem 0x12ee000000-0x138dffffff]
> [ 0.000000] NODE_DATA [mem 0xcfa42000-0xcfa72fff]
> [ 0.000000] NODE_DATA(30) on node 1
> [ 0.000000] Initmem setup node 31 [mem 0x138e000000-0x142fffffff]
> [ 0.000000] NODE_DATA [mem 0xcfa11000-0xcfa41fff]
> [ 0.000000] NODE_DATA(31) on node 1
> [ 0.000000] kvm-clock: Using msrs 4b564d01 and 4b564d00
> [ 0.000000] kvm-clock: cpu 0, msr 0:cf991001, boot clock
> [133538.294040] Zone ranges:
> [133538.294338] DMA [mem 0x00001000-0x00ffffff]
> [133538.294804] DMA32 [mem 0x01000000-0xffffffff]
> [133538.295223] Normal [mem 0x100000000-0x142fffffff]
> [133538.295670] Movable zone start for each node
>
> Looking at the code, initially I though that the problem is with:
>
> +void set_sched_clock_stable(void)
> +{
> + if (!sched_clock_stable())
> + static_key_slow_dec(&__sched_clock_stable);
> +}
> +
> +void clear_sched_clock_stable(void)
> +{
> + /* XXX worry about clock continuity */
> + if (sched_clock_stable())
> + static_key_slow_inc(&__sched_clock_stable);
> +}
>
> I think the jump label inc/dec is reversed here. We would want to inc
> it when enabling and dec when disabling, no?

I got terribly confused with that static_key trainwreck. I know I
definitely got it wrong a few times.

I helped write the jump label stuff, but the current interface is
horrible, I really couldn't figure out what is what anymore :-(

The current code seems to work for me in that my machine ends up with
sched_clock_stable() == true and when I call clear_sched_clock_stable()
it returns false and nothing explodes.

> However, trying to reverse the two didn't help. I was still seeing the
> same odd behaviour.

I got a crash when I flipped the inc and dec ;-)

> I tried doing a simple conversion to using a simple var like before,
> which looks like this:

<snip>

> This has corrected the issue:
>
> [ 0.000000] Initmem setup node 31 [mem 0x138e000000-0x142fffffff]
> [ 0.000000] NODE_DATA [mem 0xcfa11000-0xcfa41fff]
> [ 0.000000] NODE_DATA(31) on node 1
> [ 0.000000] kvm-clock: Using msrs 4b564d01 and 4b564d00
> [ 0.000000] kvm-clock: cpu 0, msr 0:cf991001, boot clock
> [ 0.000000] Zone ranges:
> [ 0.000000] DMA [mem 0x00001000-0x00ffffff]
> [ 0.000000] DMA32 [mem 0x01000000-0xffffffff]
> [ 0.000000] Normal [mem 0x100000000-0x142fffffff]
> [ 0.000000] Movable zone start for each node
> [ 0.000000] Early memory node ranges
> [ timing is correct for the rest of the boot]
>
> At this point, I thought that there's something up with jump labels
> being used this early (?) and tried compiling with
> CONFIG_JUMP_LABELS=n, this didn't solve the issue.
>
> This makes me thing there's something different related to jumplabels
> we're missing, as the no-jumplabel config should be very similar to
> the patch I did above, I just can't figure out what it is.

Does this kvm thing have sched_clock_stable==true ever? My machine ends
up setting set_sched_clock_stable() very early indeed, in
early_init_intel().

I suspect what happens is that the way I wrote it; the jump_label is
true per boot, so sched_clock_stable() == true. My initial
set_sched_clock_stable() call then does nothing as the state is already
good.

KVM doesn't do this and runs with sched_clock_stable()==true for a
while, detects the TSC really isn't stable and calls
clear_sched_clock_stable() and you get this jump in time.

Now the next patch 'fixes' this patch by adding a workqueue to delay the
actual jump_label poke; that fixed a few wierd lockdep errors.

But the above is still obviously wrong for machines that do not call
set_sched_clock_stable() on boot.

Ho humm.


\
 
 \ /
  Last update: 2014-01-22 12:21    [W:0.166 / U:1.884 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site