All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kiszka <jan.kiszka@siemens.com>
To: liu ping fan <qemulist@gmail.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Anthony Liguori <anthony@codemonkey.ws>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [PATCH] cpu: smp_wmb before lauching cpus.
Date: Fri, 06 Jul 2012 10:14:54 +0200	[thread overview]
Message-ID: <4FF69E7E.7060203@siemens.com> (raw)
In-Reply-To: <CAJnKYQ=FdfCthhoUtR7ed3qEi-o71fCP-EGSEFdOHZ7a-Jzjdg@mail.gmail.com>

On 2012-07-06 09:46, liu ping fan wrote:
> On Thu, Jul 5, 2012 at 7:58 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2012-07-05 13:02, liu ping fan wrote:
>>> On Thu, Jul 5, 2012 at 6:16 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2012-07-05 12:10, liu ping fan wrote:
>>>>> On Thu, Jul 5, 2012 at 2:46 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>>>> On 2012-07-05 04:18, Liu Ping Fan wrote:
>>>>>>> Vcpu state must be set completely before receiving INIT-IPI,SIPI
>>>>>>>
>>>>>>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>>>>>>> ---
>>>>>>>  kvm.h |    1 +
>>>>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/kvm.h b/kvm.h
>>>>>>> index 9c7b0ea..5b3c228 100644
>>>>>>> --- a/kvm.h
>>>>>>> +++ b/kvm.h
>>>>>>> @@ -198,6 +198,7 @@ static inline void cpu_synchronize_post_init(CPUArchState *env)
>>>>>>>  {
>>>>>>>      if (kvm_enabled()) {
>>>>>>>          kvm_cpu_synchronize_post_init(env);
>>>>>>> +        smp_wmb();
>>>>>>>      }
>>>>>>>  }
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> In theory, there should be no vcpu kick-off after this without some
>>>>>> locking operations involved that imply barriers. Did you see real
>>>>>
>>>>> Yeah, but what if it is non-x86?
>>>>
>>>> The locking I'm referring to is arch independent.
>>>>
>>>>>> inconsistencies without this explicit one?
>>>>
>>>> Again: Did you see real issues or is this based on static analysis?
>>>>
>>> Just on  static analysis
>>
>> Then please describe - also for the changelog - at least one case in
>> details where this is needed.
>>
> I dived into code. And yes, as you said, ACPI eject does involve some
> locking operation. So the only thing left is for starting up.  There
> seems no potential lock operation from cpu_synchronize_all_post_init()
> to  resume_all_vcpus().

There is no difference in this scenario as well: the vcpu is waiting on
a condvar for the initial kick-off and will synchronize with the main
thread via the global mutex. When in doubt, try using a debugger.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux



WARNING: multiple messages have this Message-ID (diff)
From: Jan Kiszka <jan.kiszka@siemens.com>
To: liu ping fan <qemulist@gmail.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Anthony Liguori <anthony@codemonkey.ws>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: Re: [Qemu-devel] [PATCH] cpu: smp_wmb before lauching cpus.
Date: Fri, 06 Jul 2012 10:14:54 +0200	[thread overview]
Message-ID: <4FF69E7E.7060203@siemens.com> (raw)
In-Reply-To: <CAJnKYQ=FdfCthhoUtR7ed3qEi-o71fCP-EGSEFdOHZ7a-Jzjdg@mail.gmail.com>

On 2012-07-06 09:46, liu ping fan wrote:
> On Thu, Jul 5, 2012 at 7:58 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>> On 2012-07-05 13:02, liu ping fan wrote:
>>> On Thu, Jul 5, 2012 at 6:16 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>> On 2012-07-05 12:10, liu ping fan wrote:
>>>>> On Thu, Jul 5, 2012 at 2:46 PM, Jan Kiszka <jan.kiszka@web.de> wrote:
>>>>>> On 2012-07-05 04:18, Liu Ping Fan wrote:
>>>>>>> Vcpu state must be set completely before receiving INIT-IPI,SIPI
>>>>>>>
>>>>>>> Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>>>>>>> ---
>>>>>>>  kvm.h |    1 +
>>>>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>>>>
>>>>>>> diff --git a/kvm.h b/kvm.h
>>>>>>> index 9c7b0ea..5b3c228 100644
>>>>>>> --- a/kvm.h
>>>>>>> +++ b/kvm.h
>>>>>>> @@ -198,6 +198,7 @@ static inline void cpu_synchronize_post_init(CPUArchState *env)
>>>>>>>  {
>>>>>>>      if (kvm_enabled()) {
>>>>>>>          kvm_cpu_synchronize_post_init(env);
>>>>>>> +        smp_wmb();
>>>>>>>      }
>>>>>>>  }
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> In theory, there should be no vcpu kick-off after this without some
>>>>>> locking operations involved that imply barriers. Did you see real
>>>>>
>>>>> Yeah, but what if it is non-x86?
>>>>
>>>> The locking I'm referring to is arch independent.
>>>>
>>>>>> inconsistencies without this explicit one?
>>>>
>>>> Again: Did you see real issues or is this based on static analysis?
>>>>
>>> Just on  static analysis
>>
>> Then please describe - also for the changelog - at least one case in
>> details where this is needed.
>>
> I dived into code. And yes, as you said, ACPI eject does involve some
> locking operation. So the only thing left is for starting up.  There
> seems no potential lock operation from cpu_synchronize_all_post_init()
> to  resume_all_vcpus().

There is no difference in this scenario as well: the vcpu is waiting on
a condvar for the initial kick-off and will synchronize with the main
thread via the global mutex. When in doubt, try using a debugger.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

  reply	other threads:[~2012-07-06  8:15 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-05  2:18 [PATCH] cpu: smp_wmb before lauching cpus Liu Ping Fan
2012-07-05  2:18 ` [Qemu-devel] " Liu Ping Fan
2012-07-05  6:46 ` Jan Kiszka
2012-07-05  6:46   ` [Qemu-devel] " Jan Kiszka
2012-07-05 10:10   ` liu ping fan
2012-07-05 10:10     ` [Qemu-devel] " liu ping fan
2012-07-05 10:16     ` Jan Kiszka
2012-07-05 10:16       ` [Qemu-devel] " Jan Kiszka
2012-07-05 11:02       ` liu ping fan
2012-07-05 11:02         ` [Qemu-devel] " liu ping fan
2012-07-05 11:58         ` Jan Kiszka
2012-07-05 11:58           ` [Qemu-devel] " Jan Kiszka
2012-07-06  7:46           ` liu ping fan
2012-07-06  7:46             ` [Qemu-devel] " liu ping fan
2012-07-06  8:14             ` Jan Kiszka [this message]
2012-07-06  8:14               ` Jan Kiszka

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=4FF69E7E.7060203@siemens.com \
    --to=jan.kiszka@siemens.com \
    --cc=anthony@codemonkey.ws \
    --cc=kvm@vger.kernel.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemulist@gmail.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.