All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>,
	KVM list <kvm@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	virtualization <virtualization@lists.linux-foundation.org>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Peter Anvin <hpa@zytor.com>, Davidlohr Bueso <dave@stgolabs.net>,
	Andrey Ryabinin <a.ryabinin@samsung.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Ingo Molnar <mingo@redhat.com>,
	Sasha Levin <sasha.levin@oracle.com>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	Rik van Riel <riel@redhat.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Andi Kleen <ak@linux.intel.com>,
	xen-devel@lists.xenproject.org, Dave Jones <davej@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Waiman Long <waiman.long@hp.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Andrew
Subject: Re: [PATCH] x86 spinlock: Fix memory corruption on completing completions
Date: Wed, 11 Feb 2015 18:38:38 +0100	[thread overview]
Message-ID: <20150211173838.GB28689@redhat.com> (raw)
In-Reply-To: <54DB384A.2050305@linux.vnet.ibm.com>

On 02/11, Raghavendra K T wrote:
>
> On 02/10/2015 06:56 PM, Oleg Nesterov wrote:
>
>> In this case __ticket_check_and_clear_slowpath() really needs to cmpxchg
>> the whole .head_tail. Plus obviously more boring changes. This needs a
>> separate patch even _if_ this can work.
>
> Correct, but apart from this, before doing xadd in unlock,
> we would have to make sure lsb bit is cleared so that we can live with 1
> bit overflow to tail which is unused. now either or both of head,tail
> lsb bit may be set after unlock.

Sorry, can't understand... could you spell?

If TICKET_SLOWPATH_FLAG lives in .head arch_spin_unlock() could simply do

	head = xadd(&lock->tickets.head, TICKET_LOCK_INC);

	if (head & TICKET_SLOWPATH_FLAG)
		__ticket_unlock_kick(head);

so it can't overflow to .tail?

But probably I missed your concern.



And we we do this, probably it makes sense to add something like

	bool tickets_equal(__ticket_t one, __ticket_t two)
	{
		return (one ^ two) & ~TICKET_SLOWPATH_FLAG;
	}

and change kvm_lock_spinning() to use tickets_equal(tickets.head, want), plus
it can have more users in asm/spinlock.h.

Oleg.

WARNING: multiple messages have this Message-ID (diff)
From: Oleg Nesterov <oleg@redhat.com>
To: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Jeremy Fitzhardinge <jeremy@goop.org>,
	Sasha Levin <sasha.levin@oracle.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Peter Anvin <hpa@zytor.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	Waiman Long <waiman.long@hp.com>, Dave Jones <davej@redhat.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	Paul Gortmaker <paul.gortmaker@windriver.com>,
	Andi Kleen <ak@linux.intel.com>, Jason Wang <jasowang@redhat.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	KVM list <kvm@vger.kernel.org>,
	virtualization <virtualization@lists.linux-foundation.org>,
	xen-devel@lists.xenproject.org, Rik van Riel <riel@redhat.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrey Ryabinin <a.ryabinin@samsung.com>
Subject: Re: [PATCH] x86 spinlock: Fix memory corruption on completing completions
Date: Wed, 11 Feb 2015 18:38:38 +0100	[thread overview]
Message-ID: <20150211173838.GB28689@redhat.com> (raw)
In-Reply-To: <54DB384A.2050305@linux.vnet.ibm.com>

On 02/11, Raghavendra K T wrote:
>
> On 02/10/2015 06:56 PM, Oleg Nesterov wrote:
>
>> In this case __ticket_check_and_clear_slowpath() really needs to cmpxchg
>> the whole .head_tail. Plus obviously more boring changes. This needs a
>> separate patch even _if_ this can work.
>
> Correct, but apart from this, before doing xadd in unlock,
> we would have to make sure lsb bit is cleared so that we can live with 1
> bit overflow to tail which is unused. now either or both of head,tail
> lsb bit may be set after unlock.

Sorry, can't understand... could you spell?

If TICKET_SLOWPATH_FLAG lives in .head arch_spin_unlock() could simply do

	head = xadd(&lock->tickets.head, TICKET_LOCK_INC);

	if (head & TICKET_SLOWPATH_FLAG)
		__ticket_unlock_kick(head);

so it can't overflow to .tail?

But probably I missed your concern.



And we we do this, probably it makes sense to add something like

	bool tickets_equal(__ticket_t one, __ticket_t two)
	{
		return (one ^ two) & ~TICKET_SLOWPATH_FLAG;
	}

and change kvm_lock_spinning() to use tickets_equal(tickets.head, want), plus
it can have more users in asm/spinlock.h.

Oleg.


  reply	other threads:[~2015-02-11 17:38 UTC|newest]

Thread overview: 111+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-06 14:49 [PATCH] x86 spinlock: Fix memory corruption on completing completions Raghavendra K T
2015-02-06 15:20 ` Sasha Levin
2015-02-06 15:20 ` Sasha Levin
2015-02-06 15:20   ` Sasha Levin
2015-02-06 16:15   ` Linus Torvalds
2015-02-06 16:15     ` Linus Torvalds
2015-02-06 16:15     ` Linus Torvalds
2015-02-06 17:03     ` Andrey Ryabinin
2015-02-06 17:03       ` Andrey Ryabinin
2015-02-06 17:03     ` Andrey Ryabinin
2015-02-06 17:03     ` Andrey Ryabinin
2015-02-06 16:15   ` Linus Torvalds
2015-02-08 17:14   ` Oleg Nesterov
2015-02-08 17:14     ` Oleg Nesterov
2015-02-08 17:14   ` Oleg Nesterov
2015-02-06 16:25 ` Linus Torvalds
2015-02-06 16:25 ` Linus Torvalds
2015-02-06 16:25   ` Linus Torvalds
2015-02-06 16:25   ` Linus Torvalds
2015-02-06 19:42   ` Davidlohr Bueso
2015-02-06 19:42   ` Davidlohr Bueso
2015-02-06 19:42   ` Davidlohr Bueso
2015-02-06 19:42     ` Davidlohr Bueso
2015-02-06 21:15     ` Sasha Levin
2015-02-06 21:15     ` Sasha Levin
2015-02-06 21:15       ` Sasha Levin
2015-02-06 21:15       ` Sasha Levin
2015-02-06 23:24       ` Davidlohr Bueso
2015-02-06 23:24         ` Davidlohr Bueso
2015-02-06 23:24         ` Davidlohr Bueso
2015-02-06 23:24       ` Davidlohr Bueso
2015-02-08 17:49   ` Raghavendra K T
2015-02-08 17:49   ` Raghavendra K T
2015-02-08 17:49     ` Raghavendra K T
2015-02-08 17:49     ` Raghavendra K T
2015-02-06 18:57 ` Sasha Levin
2015-02-06 18:57   ` Sasha Levin
2015-02-08 17:57   ` Raghavendra K T
2015-02-08 17:57   ` Raghavendra K T
2015-02-08 17:57   ` Raghavendra K T
2015-02-06 18:57 ` Sasha Levin
2015-02-08 21:14 ` Jeremy Fitzhardinge
2015-02-08 21:14   ` Jeremy Fitzhardinge
2015-02-09  9:34   ` Raghavendra K T
2015-02-09  9:34   ` Raghavendra K T
2015-02-09 12:02     ` Peter Zijlstra
2015-02-09 12:02       ` Peter Zijlstra
2015-02-09 12:52       ` Raghavendra K T
2015-02-09 12:52       ` Raghavendra K T
2015-02-09 12:52         ` Raghavendra K T
2015-02-10  0:53       ` Linus Torvalds
2015-02-10  0:53       ` Linus Torvalds
2015-02-10  0:53         ` Linus Torvalds
2015-02-10  0:53         ` Linus Torvalds
2015-02-10  9:30         ` Raghavendra K T
2015-02-10  9:30           ` Raghavendra K T
2015-02-10 13:18           ` Denys Vlasenko
2015-02-10 13:18           ` Denys Vlasenko
2015-02-10 13:18           ` Denys Vlasenko
2015-02-10 13:18             ` Denys Vlasenko
2015-02-10 13:20             ` Denys Vlasenko
2015-02-10 13:20               ` Denys Vlasenko
2015-02-10 13:20               ` Denys Vlasenko
2015-02-10 13:20             ` Denys Vlasenko
2015-02-10 14:24             ` Oleg Nesterov
2015-02-10 14:24             ` Oleg Nesterov
2015-02-10 14:24               ` Oleg Nesterov
2015-02-10 14:24               ` Oleg Nesterov
2015-02-10 13:23           ` Sasha Levin
2015-02-10 13:23             ` Sasha Levin
2015-02-10 13:23           ` Sasha Levin
2015-02-10 13:26           ` Oleg Nesterov
2015-02-10 13:26             ` Oleg Nesterov
2015-02-11  1:18             ` Jeremy Fitzhardinge
2015-02-11  1:18               ` Jeremy Fitzhardinge
2015-02-11  1:18               ` Jeremy Fitzhardinge
2015-02-11 17:24               ` Oleg Nesterov
2015-02-11 17:24                 ` Oleg Nesterov
2015-02-11 17:24                 ` Oleg Nesterov
2015-02-11 23:15                 ` Jeremy Fitzhardinge
2015-02-11 23:15                   ` Jeremy Fitzhardinge
2015-02-11 23:28                   ` Linus Torvalds
2015-02-11 23:28                   ` Linus Torvalds
2015-02-12  7:08                     ` Jeremy Fitzhardinge
2015-02-12  7:08                     ` Jeremy Fitzhardinge
2015-02-12 14:18                   ` Oleg Nesterov
2015-02-12 14:18                   ` Oleg Nesterov
2015-02-12 14:18                     ` Oleg Nesterov
2015-02-12 14:18                     ` Oleg Nesterov
2015-02-11 23:15                 ` Jeremy Fitzhardinge
2015-02-11 23:15                 ` Jeremy Fitzhardinge
2015-02-11 17:24               ` Oleg Nesterov
2015-02-11  1:18             ` Jeremy Fitzhardinge
2015-02-11 11:08             ` Raghavendra K T
2015-02-11 11:08             ` Raghavendra K T
2015-02-11 11:08             ` Raghavendra K T
2015-02-11 11:08               ` Raghavendra K T
2015-02-11 17:38               ` Oleg Nesterov [this message]
2015-02-11 17:38                 ` Oleg Nesterov
2015-02-11 18:38                 ` Raghavendra K T
2015-02-11 18:38                 ` Raghavendra K T
2015-02-11 18:38                 ` Raghavendra K T
2015-02-11 18:38                   ` Raghavendra K T
2015-02-11 17:38               ` Oleg Nesterov
2015-02-10 13:26           ` Oleg Nesterov
2015-02-10  9:30         ` Raghavendra K T
2015-02-09 12:02     ` Peter Zijlstra
2015-02-09  9:34   ` Raghavendra K T
2015-02-08 21:14 ` Jeremy Fitzhardinge
  -- strict thread matches above, loose matches on Subject: below --
2015-02-06 14:49 Raghavendra K T
2015-02-06 14:49 Raghavendra K T

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150211173838.GB28689@redhat.com \
    --to=oleg@redhat.com \
    --cc=a.ryabinin@samsung.com \
    --cc=ak@linux.intel.com \
    --cc=borntraeger@de.ibm.com \
    --cc=dave@stgolabs.net \
    --cc=davej@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jeremy@goop.org \
    --cc=konrad.wilk@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=paul.gortmaker@windriver.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=raghavendra.kt@linux.vnet.ibm.com \
    --cc=riel@redhat.com \
    --cc=sasha.levin@oracle.com \
    --cc=tglx@linutronix.de \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=waiman.long@hp.com \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.