All of lore.kernel.org
 help / color / mirror / Atom feed
From: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>, KVM <kvm@vger.kernel.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	the arch/x86 maintainers <x86@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Virtualization <virtualization@lists.linux-foundation.org>,
	Andi Kleen <andi@firstfloor.org>,
	Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>,
	Avi Kivity <avi@redhat.com>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>,
	"H. Peter Anvin" <hpa@zytor.com>,
	Attilio Rao <attilio.rao@citrix.com>, Ingo Molnar <mingo@elte.hu>,
	Xen Devel <xen-devel@lists.xensource.com>,
	Stephan Diestelhorst <stephan.diestelhorst@amd.com>
Subject: Re: [PATCH RFC V6 2/11] x86/ticketlock: don't inline _spin_unlock when using paravirt spinlocks
Date: Thu, 22 Mar 2012 15:36:42 +0530	[thread overview]
Message-ID: <4F6AF9B2.4070207@linux.vnet.ibm.com> (raw)
In-Reply-To: <CA+55aFzJn_U=2-kxGX15v6zLaCV+Jgtxjop2PudizzsqSn+nLg@mail.gmail.com>

On 03/21/2012 10:43 PM, Linus Torvalds wrote:
> On Wed, Mar 21, 2012 at 3:21 AM, Raghavendra K T
> <raghavendra.kt@linux.vnet.ibm.com>  wrote:
>> From: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
>>
>> The code size expands somewhat, and its probably better to just call
>> a function rather than inline it.
>>
>> Signed-off-by: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
>> Signed-off-by: Raghavendra K T<raghavendra.kt@linux.vnet.ibm.com>
>> ---
>>   arch/x86/Kconfig     |    3 +++
>>   kernel/Kconfig.locks |    2 +-
>>   2 files changed, 4 insertions(+), 1 deletions(-)
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 5bed94e..10c28ec 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -623,6 +623,9 @@ config PARAVIRT_SPINLOCKS
>>
>>           If you are unsure how to answer this question, answer N.
>>
>> +config ARCH_NOINLINE_SPIN_UNLOCK
>> +       def_bool PARAVIRT_SPINLOCKS
>> +
>>   config PARAVIRT_CLOCK
>>         bool
>>
>> diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks
>> index 5068e2a..584637b 100644
>> --- a/kernel/Kconfig.locks
>> +++ b/kernel/Kconfig.locks
>> @@ -125,7 +125,7 @@ config INLINE_SPIN_LOCK_IRQSAVE
>>                  ARCH_INLINE_SPIN_LOCK_IRQSAVE
>>
>>   config INLINE_SPIN_UNLOCK
>> -       def_bool !DEBUG_SPINLOCK&&  (!PREEMPT || ARCH_INLINE_SPIN_UNLOCK)
>> +       def_bool !DEBUG_SPINLOCK&&  (!PREEMPT || ARCH_INLINE_SPIN_UNLOCK)&&  !ARCH_NOINLINE_SPIN_UNLOCK
>>
>>   config INLINE_SPIN_UNLOCK_BH
>>         def_bool !DEBUG_SPINLOCK&&  ARCH_INLINE_SPIN_UNLOCK_BH
>
> Ugh. This is getting really ugly.
>

Agree that it had become longer.

> Can we just fix it by
>   - getting rid of INLINE_SPIN_UNLOCK entirely
>
>   - replacing it with UNINLINE_SPIN_UNLOCK instead with the reverse
> meaning, and no "def_bool" at all, just a simple
>
>          config UNINLINE_SPIN_UNLOCK
>                  bool
>
>   - make the various people who want to uninline the spinlocks (like
> spinlock debugging, paravirt etc) all just do
>
>          select UNINLINE_SPIN_UNLOCK

I just posted  https://lkml.org/lkml/2012/3/22/94. Please let me know
if that looks better.
And this patch should now become something like
---
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5bed94e..2666b7d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -613,6 +613,7 @@ config PARAVIRT
  config PARAVIRT_SPINLOCKS
	 bool "Paravirtualization layer for spinlocks"
	 depends on PARAVIRT && SMP && EXPERIMENTAL
+	 select UNINLINE_SPIN_UNLOCK
	 ---help---
	   Paravirtualized spinlocks allow a pvops backend to replace the
	   spinlock implementation with something virtualization-friendly

WARNING: multiple messages have this Message-ID (diff)
From: Raghavendra K T <raghavendra.kt@linux.vnet.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <peterz@infradead.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	LKML <linux-kernel@vger.kernel.org>, Avi Kivity <avi@redhat.com>,
	Marcelo Tosatti <mtosatti@redhat.com>, KVM <kvm@vger.kernel.org>,
	Andi Kleen <andi@firstfloor.org>,
	Xen Devel <xen-devel@lists.xensource.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Virtualization <virtualization@lists.linux-foundation.org>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>,
	Stephan Diestelhorst <stephan.diestelhorst@amd.com>,
	Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Attilio Rao <attilio.rao@citrix.com>
Subject: Re: [PATCH RFC V6 2/11] x86/ticketlock: don't inline _spin_unlock when using paravirt spinlocks
Date: Thu, 22 Mar 2012 15:36:42 +0530	[thread overview]
Message-ID: <4F6AF9B2.4070207@linux.vnet.ibm.com> (raw)
In-Reply-To: <CA+55aFzJn_U=2-kxGX15v6zLaCV+Jgtxjop2PudizzsqSn+nLg@mail.gmail.com>

On 03/21/2012 10:43 PM, Linus Torvalds wrote:
> On Wed, Mar 21, 2012 at 3:21 AM, Raghavendra K T
> <raghavendra.kt@linux.vnet.ibm.com>  wrote:
>> From: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
>>
>> The code size expands somewhat, and its probably better to just call
>> a function rather than inline it.
>>
>> Signed-off-by: Jeremy Fitzhardinge<jeremy.fitzhardinge@citrix.com>
>> Signed-off-by: Raghavendra K T<raghavendra.kt@linux.vnet.ibm.com>
>> ---
>>   arch/x86/Kconfig     |    3 +++
>>   kernel/Kconfig.locks |    2 +-
>>   2 files changed, 4 insertions(+), 1 deletions(-)
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 5bed94e..10c28ec 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -623,6 +623,9 @@ config PARAVIRT_SPINLOCKS
>>
>>           If you are unsure how to answer this question, answer N.
>>
>> +config ARCH_NOINLINE_SPIN_UNLOCK
>> +       def_bool PARAVIRT_SPINLOCKS
>> +
>>   config PARAVIRT_CLOCK
>>         bool
>>
>> diff --git a/kernel/Kconfig.locks b/kernel/Kconfig.locks
>> index 5068e2a..584637b 100644
>> --- a/kernel/Kconfig.locks
>> +++ b/kernel/Kconfig.locks
>> @@ -125,7 +125,7 @@ config INLINE_SPIN_LOCK_IRQSAVE
>>                  ARCH_INLINE_SPIN_LOCK_IRQSAVE
>>
>>   config INLINE_SPIN_UNLOCK
>> -       def_bool !DEBUG_SPINLOCK&&  (!PREEMPT || ARCH_INLINE_SPIN_UNLOCK)
>> +       def_bool !DEBUG_SPINLOCK&&  (!PREEMPT || ARCH_INLINE_SPIN_UNLOCK)&&  !ARCH_NOINLINE_SPIN_UNLOCK
>>
>>   config INLINE_SPIN_UNLOCK_BH
>>         def_bool !DEBUG_SPINLOCK&&  ARCH_INLINE_SPIN_UNLOCK_BH
>
> Ugh. This is getting really ugly.
>

Agree that it had become longer.

> Can we just fix it by
>   - getting rid of INLINE_SPIN_UNLOCK entirely
>
>   - replacing it with UNINLINE_SPIN_UNLOCK instead with the reverse
> meaning, and no "def_bool" at all, just a simple
>
>          config UNINLINE_SPIN_UNLOCK
>                  bool
>
>   - make the various people who want to uninline the spinlocks (like
> spinlock debugging, paravirt etc) all just do
>
>          select UNINLINE_SPIN_UNLOCK

I just posted  https://lkml.org/lkml/2012/3/22/94. Please let me know
if that looks better.
And this patch should now become something like
---
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5bed94e..2666b7d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -613,6 +613,7 @@ config PARAVIRT
  config PARAVIRT_SPINLOCKS
	 bool "Paravirtualization layer for spinlocks"
	 depends on PARAVIRT && SMP && EXPERIMENTAL
+	 select UNINLINE_SPIN_UNLOCK
	 ---help---
	   Paravirtualized spinlocks allow a pvops backend to replace the
	   spinlock implementation with something virtualization-friendly


  reply	other threads:[~2012-03-22 10:06 UTC|newest]

Thread overview: 113+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-21 10:20 [PATCH RFC V6 0/11] Paravirtualized ticketlocks Raghavendra K T
2012-03-21 10:20 ` Raghavendra K T
2012-03-21 10:20 ` [PATCH RFC V6 1/11] x86/spinlock: replace pv spinlocks with pv ticketlocks Raghavendra K T
2012-03-21 10:20   ` Raghavendra K T
2012-03-21 13:04   ` Attilio Rao
2012-03-21 13:22     ` Stephan Diestelhorst
2012-03-21 13:22     ` Stephan Diestelhorst
2012-03-21 13:49       ` Attilio Rao
2012-03-21 14:25         ` Stephan Diestelhorst
2012-03-21 14:33           ` Attilio Rao
2012-03-21 14:33           ` Attilio Rao
2012-03-21 14:49             ` Raghavendra K T
2012-03-21 14:49             ` Raghavendra K T
2012-03-21 14:25         ` Stephan Diestelhorst
2012-03-21 13:49       ` Attilio Rao
2012-03-21 13:04   ` Attilio Rao
2012-03-21 10:21 ` [PATCH RFC V6 2/11] x86/ticketlock: don't inline _spin_unlock when using paravirt spinlocks Raghavendra K T
2012-03-21 17:13   ` Linus Torvalds
2012-03-21 17:13     ` Linus Torvalds
2012-03-22 10:06     ` Raghavendra K T [this message]
2012-03-22 10:06       ` Raghavendra K T
2012-03-22 10:06     ` Raghavendra K T
2012-03-21 10:21 ` Raghavendra K T
2012-03-21 10:21 ` [PATCH RFC V6 3/11] x86/ticketlock: collapse a layer of functions Raghavendra K T
2012-03-21 10:21 ` Raghavendra K T
2012-03-21 10:21 ` [PATCH RFC V6 4/11] xen: defer spinlock setup until boot CPU setup Raghavendra K T
2012-03-21 10:21   ` Raghavendra K T
2012-03-21 10:21 ` [PATCH RFC V6 5/11] xen/pvticketlock: Xen implementation for PV ticket locks Raghavendra K T
2012-03-21 10:21   ` Raghavendra K T
2012-03-21 10:21 ` [PATCH RFC V6 6/11] xen/pvticketlocks: add xen_nopvspin parameter to disable xen pv ticketlocks Raghavendra K T
2012-03-21 10:21 ` Raghavendra K T
2012-03-21 10:21 ` [PATCH RFC V6 7/11] x86/pvticketlock: use callee-save for lock_spinning Raghavendra K T
2012-03-21 10:21   ` Raghavendra K T
2012-03-21 10:22 ` [PATCH RFC V6 8/11] x86/pvticketlock: when paravirtualizing ticket locks, increment by 2 Raghavendra K T
2012-03-21 10:22 ` Raghavendra K T
2012-03-21 10:22   ` Raghavendra K T
2012-03-21 10:22 ` [PATCH RFC V6 9/11] x86/ticketlock: add slowpath logic Raghavendra K T
2012-03-21 10:22 ` Raghavendra K T
2012-03-21 10:22 ` [PATCH RFC V6 10/11] xen/pvticketlock: allow interrupts to be enabled while blocking Raghavendra K T
2012-03-21 10:22   ` Raghavendra K T
2012-03-21 10:22 ` [PATCH RFC V6 11/11] xen: enable PV ticketlocks on HVM Xen Raghavendra K T
2012-03-21 10:22   ` Raghavendra K T
2012-03-26 14:25 ` [PATCH RFC V6 0/11] Paravirtualized ticketlocks Avi Kivity
2012-03-26 14:25   ` Avi Kivity
2012-03-27  7:37   ` Raghavendra K T
2012-03-27  7:37   ` Raghavendra K T
2012-03-28 16:09     ` Alan Meadows
2012-03-28 18:21       ` Raghavendra K T
2012-03-28 18:21         ` Raghavendra K T
2012-03-29  9:58         ` Avi Kivity
2012-03-29  9:58           ` Avi Kivity
2012-03-29 18:03           ` Raghavendra K T
2012-03-29 18:03             ` Raghavendra K T
2012-03-30 10:07             ` Raghavendra K T
2012-03-30 10:07             ` Raghavendra K T
2012-04-01 13:18               ` Avi Kivity
2012-04-01 13:18                 ` Avi Kivity
2012-04-01 13:48                 ` Raghavendra K T
2012-04-01 13:48                   ` Raghavendra K T
2012-04-01 13:53                   ` Avi Kivity
2012-04-01 13:53                     ` Avi Kivity
2012-04-01 13:56                     ` Raghavendra K T
2012-04-01 13:56                       ` Raghavendra K T
2012-04-02  9:51                     ` Raghavendra K T
2012-04-02  9:51                     ` Raghavendra K T
2012-04-02 12:15                       ` Raghavendra K T
2012-04-02 12:15                       ` Raghavendra K T
2012-04-05  9:01                       ` Avi Kivity
2012-04-05  9:01                         ` Avi Kivity
2012-04-05 10:40                         ` Raghavendra K T
2012-04-05 10:40                         ` Raghavendra K T
2012-04-02  9:51                     ` Raghavendra K T
2012-04-05  8:43                     ` Raghavendra K T
2012-04-05  8:43                       ` Raghavendra K T
2012-03-28 16:37     ` Alan Meadows
2012-03-30 20:26 ` H. Peter Anvin
2012-03-30 20:26   ` H. Peter Anvin
2012-03-30 22:07   ` Thomas Gleixner
2012-03-30 22:07     ` Thomas Gleixner
2012-03-30 22:18     ` Andi Kleen
2012-03-30 22:18       ` Andi Kleen
2012-03-30 23:04       ` Thomas Gleixner
2012-03-30 23:04         ` Thomas Gleixner
2012-03-31  0:08         ` Andi Kleen
2012-03-31  0:08           ` Andi Kleen
2012-03-31  8:11           ` Ingo Molnar
2012-03-31  8:11           ` Ingo Molnar
2012-03-31  4:07     ` Srivatsa Vaddagiri
2012-03-31  4:07       ` Srivatsa Vaddagiri
2012-03-31  4:09       ` Srivatsa Vaddagiri
2012-03-31  4:09         ` Srivatsa Vaddagiri
2012-04-16 15:44       ` Konrad Rzeszutek Wilk
2012-04-16 15:44         ` Konrad Rzeszutek Wilk
2012-04-16 16:36         ` [Xen-devel] " Ian Campbell
2012-04-16 16:36           ` Ian Campbell
2012-04-16 16:42           ` Jeremy Fitzhardinge
2012-04-16 16:42           ` Jeremy Fitzhardinge
2012-04-17  2:54           ` Srivatsa Vaddagiri
2012-04-17  2:54             ` Srivatsa Vaddagiri
2012-04-01 13:31     ` Avi Kivity
2012-04-01 13:31       ` Avi Kivity
2012-04-02  9:26       ` Thomas Gleixner
2012-04-02  9:26         ` Thomas Gleixner
2012-04-05  9:15         ` Avi Kivity
2012-04-05  9:15           ` Avi Kivity
2012-04-02  4:36     ` [Xen-devel] " Juergen Gross
2012-04-02  4:36     ` Juergen Gross
2012-04-02  9:42     ` Ian Campbell
2012-04-02  9:42       ` Ian Campbell
2012-04-11  1:29     ` Marcelo Tosatti
2012-04-11  1:29       ` Marcelo Tosatti
2012-03-31  0:51   ` Raghavendra K T
2012-03-31  0:51     ` 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=4F6AF9B2.4070207@linux.vnet.ibm.com \
    --to=raghavendra.kt@linux.vnet.ibm.com \
    --cc=andi@firstfloor.org \
    --cc=attilio.rao@citrix.com \
    --cc=avi@redhat.com \
    --cc=hpa@zytor.com \
    --cc=jeremy.fitzhardinge@citrix.com \
    --cc=konrad.wilk@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mtosatti@redhat.com \
    --cc=peterz@infradead.org \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=stephan.diestelhorst@amd.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vatsa@linux.vnet.ibm.com \
    --cc=virtualization@lists.linux-foundation.org \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xensource.com \
    /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.