Re: [PATCH 9/9] qspinlock, x86, kvm: Implement KVM support for paravirt qspinlock

Waiman Long <waiman.long@xxxxxx> · Wed, 18 Mar 2015 22:45:55 -0400

    On 03/16/2015 09:16 AM, Peter Zijlstra wrote:

        Implement the paravirt qspinlock for x86-kvm.

We use the regular paravirt call patching to switch between:

  native_queue_spin_lock_slowpath()	__pv_queue_spin_lock_slowpath()
  native_queue_spin_unlock()		__pv_queue_spin_unlock()

We use a callee saved call for the unlock function which reduces the
i-cache footprint and allows 'inlining' of SPIN_UNLOCK functions
again.

We further optimize the unlock path by patching the direct call with a
"movb $0,%arg1" if we are indeed using the native unlock code. This
makes the unlock code almost as fast as the !PARAVIRT case.

This significantly lowers the overhead of having
CONFIG_PARAVIRT_SPINLOCKS enabled, even for native code.

Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>

    I do have some concern about this call site patching mechanism as
    the modification is not atomic. The spin_unlock() calls are in many
    places in the kernel. There is a possibility that a thread is
    calling a certain spin_unlock call site while it is being patched by
    another one with the alternative() function call. 

    So far, I don't see any problem with bare metal where
    paravirt_patch_insns() is used to patch it to the move instruction.
    However, in a virtual guest enivornment where paravirt_patch_call()
    was used, there were situations where the system panic because of
    page fault on some invalid memory in the kthread. If you look at the
    paravirt_patch_call(), you will see:

        :

    b->opcode = 0xe8; /* call */

    b->delta = delta;

    If another CPU reads the instruction at the call site at the right
    moment, it will get the modified call instruction, but not the new
    delta value. It will then jump to a random location. I believe that
    was causing the system panic that I saw.

    So I think it is kind of risky to use it here unless we can
    guarantee that call site patching is atomic wrt other CPUs.

    -Longman

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization