From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1760810AbbBIPRS (ORCPT <rfc822;w@1wt.eu>);
	Mon, 9 Feb 2015 10:17:18 -0500
Received: from mx1.redhat.com ([209.132.183.28]:33994 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1759898AbbBIPRR (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Mon, 9 Feb 2015 10:17:17 -0500
Date: Mon, 9 Feb 2015 16:14:59 +0100
From: Oleg Nesterov <oleg@redhat.com>
To: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
Cc: tglx@linutronix.de, mingo@redhat.com, hpa@zytor.com, peterz@infradead.org,
        torvalds@linux-foundation.org, konrad.wilk@oracle.com,
        pbonzini@redhat.com, paulmck@linux.vnet.ibm.com, waiman.long@hp.com,
        davej@redhat.com, x86@kernel.org, jeremy@goop.org,
        paul.gortmaker@windriver.com, ak@linux.intel.com, jasowang@redhat.com,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        virtualization@lists.linux-foundation.org,
        xen-devel@lists.xenproject.org, riel@redhat.com,
        borntraeger@de.ibm.com, akpm@linux-foundation.org,
        a.ryabinin@samsung.com, sasha.levin@oracle.com
Subject: Re: [PATCH V2] x86 spinlock: Fix memory corruption on completing
	completions
Message-ID: <20150209151459.GA973@redhat.com>
References: <1423485611-18369-1-git-send-email-raghavendra.kt@linux.vnet.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1423485611-18369-1-git-send-email-raghavendra.kt@linux.vnet.ibm.com>
User-Agent: Mutt/1.5.18 (2008-05-17)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 02/09, Raghavendra K T wrote:
>
> +static inline void __ticket_check_and_clear_slowpath(arch_spinlock_t *lock)
> +{
> +	arch_spinlock_t old, new;
> +	__ticket_t diff;
> +
> +	old.tickets = READ_ONCE(lock->tickets);
> +	diff = (old.tickets.tail & ~TICKET_SLOWPATH_FLAG) - old.tickets.head;
> +
> +	/* try to clear slowpath flag when there are no contenders */
> +	if ((old.tickets.tail & TICKET_SLOWPATH_FLAG) &&
> +		(diff == TICKET_LOCK_INC)) {
> +		new = old;
> +		new.tickets.tail &= ~TICKET_SLOWPATH_FLAG;
> +		cmpxchg(&lock->head_tail, old.head_tail, new.head_tail);
> +	}
> +}

Can't we simplify it? We own .head, and we already know it. We only need
to clear TICKET_SLOWPATH_FLAG in .tail atomically?

IOW,

	static inline void __ticket_check_and_clear_slowpath(arch_spinlock_t *lock, __ticket_t head)
	{
		__ticket_t old_tail, new_tail;

		new_tail = head + TICKET_LOCK_INC;
		old_tail = new_tail | TICKET_SLOWPATH_FLAG;

		if (READ_ONCE(lock->tickets.tail) == old_tail)
			cmpxchg(&lock->tickets.tail, old_tail, new_tail);
	}

Plus

	-	__ticket_check_and_clear_slowpath(lock);
	+	__ticket_check_and_clear_slowpath(lock, inc.tail);

Or I missed something?

And I think it would be better to avoid ifdef(CONFIG_PARAVIRT_SPINLOCKS),
ww can just do

	if (TICKET_SLOWPATH_FLAG)
		__ticket_check_and_clear_slowpath();

Oleg.