All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] ipmi: do not take/drop iothread lock
@ 2016-01-26 16:15 Paolo Bonzini
  2016-01-26 16:21 ` Michael S. Tsirkin
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2016-01-26 16:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: cminyard, mst

This is not necessary and actually causes a hang; it was probably copied
and pasted from KVM code, that is one of the very few places that run
outside iothread lock.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ipmi/ipmi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index 52aba1e..fcba0ca 100644
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -50,9 +50,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
         if (checkonly) {
             return 0;
         }
-        qemu_mutex_lock_iothread();
         qmp_inject_nmi(NULL);
-        qemu_mutex_unlock_iothread();
         return 0;
 
     case IPMI_POWERCYCLE_CHASSIS:
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] ipmi: do not take/drop iothread lock
  2016-01-26 16:15 [Qemu-devel] [PATCH] ipmi: do not take/drop iothread lock Paolo Bonzini
@ 2016-01-26 16:21 ` Michael S. Tsirkin
  2016-01-26 17:57   ` Corey Minyard
  0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2016-01-26 16:21 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: cminyard, qemu-devel

On Tue, Jan 26, 2016 at 05:15:53PM +0100, Paolo Bonzini wrote:
> This is not necessary and actually causes a hang; it was probably copied
> and pasted from KVM code, that is one of the very few places that run
> outside iothread lock.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Makes sense. I wonder how hard would it be to have a test to catch this.

> ---
>  hw/ipmi/ipmi.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
> index 52aba1e..fcba0ca 100644
> --- a/hw/ipmi/ipmi.c
> +++ b/hw/ipmi/ipmi.c
> @@ -50,9 +50,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
>          if (checkonly) {
>              return 0;
>          }
> -        qemu_mutex_lock_iothread();
>          qmp_inject_nmi(NULL);
> -        qemu_mutex_unlock_iothread();
>          return 0;
>  
>      case IPMI_POWERCYCLE_CHASSIS:
> -- 
> 2.5.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [Qemu-devel] [PATCH] ipmi: do not take/drop iothread lock
@ 2016-01-26 16:59 Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2016-01-26 16:59 UTC (permalink / raw)
  To: qemu-devel; +Cc: stefanha

This is not necessary and actually causes a hang; it was probably copied
and pasted from KVM code, that is one of the very few places that run
outside iothread lock.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/ipmi/ipmi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
index 52aba1e..fcba0ca 100644
--- a/hw/ipmi/ipmi.c
+++ b/hw/ipmi/ipmi.c
@@ -50,9 +50,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
         if (checkonly) {
             return 0;
         }
-        qemu_mutex_lock_iothread();
         qmp_inject_nmi(NULL);
-        qemu_mutex_unlock_iothread();
         return 0;
 
     case IPMI_POWERCYCLE_CHASSIS:
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] ipmi: do not take/drop iothread lock
  2016-01-26 16:21 ` Michael S. Tsirkin
@ 2016-01-26 17:57   ` Corey Minyard
  2016-01-26 18:00     ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Corey Minyard @ 2016-01-26 17:57 UTC (permalink / raw)
  To: Michael S. Tsirkin, Paolo Bonzini; +Cc: qemu-devel

On 01/26/2016 10:21 AM, Michael S. Tsirkin wrote:
> On Tue, Jan 26, 2016 at 05:15:53PM +0100, Paolo Bonzini wrote:
>> This is not necessary and actually causes a hang; it was probably copied
>> and pasted from KVM code, that is one of the very few places that run
>> outside iothread lock.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Makes sense. I wonder how hard would it be to have a test to catch this.

The IPMI side would be easy for me, but I don't know how to handle 
catching the NMI.

-corey

>> ---
>>   hw/ipmi/ipmi.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
>> index 52aba1e..fcba0ca 100644
>> --- a/hw/ipmi/ipmi.c
>> +++ b/hw/ipmi/ipmi.c
>> @@ -50,9 +50,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
>>           if (checkonly) {
>>               return 0;
>>           }
>> -        qemu_mutex_lock_iothread();
>>           qmp_inject_nmi(NULL);
>> -        qemu_mutex_unlock_iothread();
>>           return 0;
>>   
>>       case IPMI_POWERCYCLE_CHASSIS:
>> -- 
>> 2.5.0

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] ipmi: do not take/drop iothread lock
  2016-01-26 17:57   ` Corey Minyard
@ 2016-01-26 18:00     ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2016-01-26 18:00 UTC (permalink / raw)
  To: Corey Minyard, Michael S. Tsirkin; +Cc: qemu-devel



On 26/01/2016 18:57, Corey Minyard wrote:
> On 01/26/2016 10:21 AM, Michael S. Tsirkin wrote:
>> On Tue, Jan 26, 2016 at 05:15:53PM +0100, Paolo Bonzini wrote:
>>> This is not necessary and actually causes a hang; it was probably copied
>>> and pasted from KVM code, that is one of the very few places that run
>>> outside iothread lock.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> Makes sense. I wonder how hard would it be to have a test to catch this.
> 
> The IPMI side would be easy for me, but I don't know how to handle
> catching the NMI.

I'm afraid there's no way to trap an NMI from qtest.

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-01-26 18:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-26 16:15 [Qemu-devel] [PATCH] ipmi: do not take/drop iothread lock Paolo Bonzini
2016-01-26 16:21 ` Michael S. Tsirkin
2016-01-26 17:57   ` Corey Minyard
2016-01-26 18:00     ` Paolo Bonzini
  -- strict thread matches above, loose matches on Subject: below --
2016-01-26 16:59 Paolo Bonzini

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.