Skip to content
  • Raghavendra K T's avatar
    x86/spinlocks/paravirt: Fix memory corruption on unlock · d6abfdb2
    Raghavendra K T authored
    
    
    Paravirt spinlock clears slowpath flag after doing unlock.
    As explained by Linus currently it does:
    
                    prev = *lock;
                    add_smp(&lock->tickets.head, TICKET_LOCK_INC);
    
                    /* add_smp() is a full mb() */
    
                    if (unlikely(lock->tickets.tail & TICKET_SLOWPATH_FLAG))
                            __ticket_unlock_slowpath(lock, prev);
    
    which is *exactly* the kind of things you cannot do with spinlocks,
    because after you've done the "add_smp()" and released the spinlock
    for the fast-path, you can't access the spinlock any more.  Exactly
    because a fast-path lock might come in, and release the whole data
    structure.
    
    Linus suggested that we should not do any writes to lock after unlock(),
    and we can move slowpath clearing to fastpath lock.
    
    So this patch implements the fix with:
    
     1. Moving slowpath flag to head (Oleg):
        Unlocked locks don't care about the slowpath flag; therefore we can keep
        it set after the last unlock, and clear it again on the first (try)lock.
        -- this removes the write after unlock. note that keeping slowpath flag would
        result in unnecessary kicks.
        By moving the slowpath flag from the tail to the head ticket we also avoid
        the need to access both the head and tail tickets on unlock.
    
     2. use xadd to avoid read/write after unlock that checks the need for
        unlock_kick (Linus):
        We further avoid the need for a read-after-release by using xadd;
        the prev head value will include the slowpath flag and indicate if we
        need to do PV kicking of suspended spinners -- on modern chips xadd
        isn't (much) more expensive than an add + load.
    
    Result:
     setup: 16core (32 cpu +ht sandy bridge 8GB 16vcpu guest)
     benchmark overcommit %improve
     kernbench  1x           -0.13
     kernbench  2x            0.02
     dbench     1x           -1.77
     dbench     2x           -0.63
    
    [Jeremy: Hinted missing TICKET_LOCK_INC for kick]
    [Oleg: Moved slowpath flag to head, ticket_equals idea]
    [PeterZ: Added detailed changelog]
    
    Suggested-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    Reported-by: default avatarSasha Levin <sasha.levin@oracle.com>
    Tested-by: default avatarSasha Levin <sasha.levin@oracle.com>
    Signed-off-by: default avatarRaghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
    Signed-off-by: default avatarPeter Zijlstra (Intel) <peterz@infradead.org>
    Reviewed-by: default avatarOleg Nesterov <oleg@redhat.com>
    Cc: Andrew Jones <drjones@redhat.com>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Cc: Andy Lutomirski <luto@amacapital.net>
    Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
    Cc: Christian Borntraeger <borntraeger@de.ibm.com>
    Cc: Christoph Lameter <cl@linux.com>
    Cc: Dave Hansen <dave.hansen@linux.intel.com>
    Cc: Dave Jones <davej@redhat.com>
    Cc: David Vrabel <david.vrabel@citrix.com>
    Cc: Fernando Luis Vázquez Cao <fernando_b1@lab.ntt.co.jp>
    Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
    Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
    Cc: Paolo Bonzini <pbonzini@redhat.com>
    Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Ulrich Obergfell <uobergfe@redhat.com>
    Cc: Waiman Long <Waiman.Long@hp.com>
    Cc: a.ryabinin@samsung.com
    Cc: dave@stgolabs.net
    Cc: hpa@zytor.com
    Cc: jasowang@redhat.com
    Cc: jeremy@goop.org
    Cc: paul.gortmaker@windriver.com
    Cc: riel@redhat.com
    Cc: tglx@linutronix.de
    Cc: waiman.long@hp.com
    Cc: xen-devel@lists.xenproject.org
    Link: http://lkml.kernel.org/r/20150215173043.GA7471@linux.vnet.ibm.com
    
    
    Signed-off-by: default avatarIngo Molnar <mingo@kernel.org>
    d6abfdb2