Re: Please review arch/x86/kernel/pvclock.c to fix Docker/Mono crashes in new Kernels

[Andy Lutomirski]

On Mon, May 16, 2016 at 11:10 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
> There is something odd being reported in Ubuntu.
>
> There's a Mono SIGSEGV that was bisected to Andy's commit 1ddf0b1b11aa
> ("x86, vdso: Use asm volatile in __getcpu"), and then reported to be
> fixed with commits

I'm reasonably confident that the addition of "volatile" is not the
root cause...

>
>   80f7fdb1c7f0 ("x86: vdso: fix pvclock races with task migration")
>   0a4e6be9ca17 ("x86: kvm: Revert "remove sched notifier for cross-cpu
> migrations"")
>
> and when those were backported all looked well.
>
> But then those two commits in turn were reverted with
>
>   73459e2a1ada ("x86: pvclock: Really remove the sched notifier for
> cross-cpu migrations")
>
> and people seem to report that it's back as a result:
>
>   https://bugzilla.xamarin.com/show_bug.cgi?id=29212#c16
>
> so apparently that task migration notifier somehow does matter.

All of those fixes were intended to fix incorrect times being
reported, not segfaults.  Weird.

I tried to sign up for Xamarin bugzilla and made zero progress (the
confirmation email was never sent).  That being said, this bug report
is very confusing.  Could someone who can reproduce this provide the
following information:

1. What is the contents of
/sys/devices/system/clocksource/clocksource0/current_clocksource

2. If you do:

 echo tsc >/sys/devices/system/clocksource/clocksource0/current_clocksource

can you still reproduce it?

3. I rewrote the whole vdso pvclock mess in Linux 4.5.  Does the bug
exist in Linux 4.5?

4. What is actually crashing?  The stack trace says:

Method (wrapper managed-to-managed) string:.ctor (char[],int,int)
emitted at 0x40b5b1b0 to 0x40b5b1d9 (code length 41)

[bug-18026.exe]
converting method (wrapper managed-to-native)
object:__icall_wrapper_mono_gc_alloc_string (intptr,intptr,int)
Method (wrapper managed-to-native)
object:__icall_wrapper_mono_gc_alloc_string (intptr,intptr,int)
emitted at 0x40b5b1f0 to 0x40b5b284 (code length 148) [bug-18026.exe]

Unhandled Exception:
System.NullReferenceException: Object reference not set to an instance
of an object
  at Test.Main () [0x00000] in <filename unknown>:0
[ERROR] FATAL UNHANDLED EXCEPTION: System.NullReferenceException:
Object reference not set to an instance of an object
  at Test.Main () [0x00000] in <filename unknown>:0


What on earth does that mean?  Is mono crashing in the vdso?  Is mono
crashing because time went backwards?  Is mono crashing because its GC
is just weirdly buggy, uses clock_gettime, and has a race condition
that is or is not triggered depending on how long the function takes?

An actual stack dump of the segfault (the native stack, not what mono
thinks the stack is) would be nice.


FWIW, the pvclock host code is complicated and it's not obvious to me
that it has any particular guarantee of monotonicity.  (That's not to
say it has a bug that breaks monotonicity -- it's just that I, as a
reader of the code, have never had a clear understanding of what it's
trying to do or why it's trying to do it.)

If it's fixed in 4.5, I suppose the big rewrite could be backported,
but I'd rather have some understanding of what's going on.

--Andy

>
> Comments?
>
>                   Linus
>
>
> On Mon, May 16, 2016 at 1:13 AM,  <okhalzov@xxxxxxxxxxxxxxxxx> wrote:
>>
>> Hello Linus,
>>
>> Am am sorry to bother you, but it seems that the bug from old kernels was
>> copied to new >=4.1 kernels. We use Ubuntu/Docker/Mono and we had to
>> rollback to 3.19.0-54 kernel for the work around.
>>
>> We found that a year ago there was a discussion on the launchpad
>> (https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1450584) regarding
>> SIGSEGV on multi-cpu vm.
>> It seems to me that the commits around that bug
>> https://github.com/torvalds/linux/commits/master/arch/x86/kernel/pvclock.c
>> caused 4.1 and up kernels to keep that bug.
>> Please review pvclock.c to fix that problem.
>>
>> Kiitos! Thank you!
>>
>> --
>> Oleg Khalzov
>> SDE
>> Vestbery
>>



-- 
Andy Lutomirski
AMA Capital Management, LLC