diff mbox series

Revert "irqbypass: do not start cons/prod when failed connect"

Message ID 20210508071152.722425-1-lingshan.zhu@intel.com (mailing list archive)
State New, archived
Headers show
Series Revert "irqbypass: do not start cons/prod when failed connect" | expand

Commit Message

Zhu Lingshan May 8, 2021, 7:11 a.m. UTC
This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.

The reverted commit may cause VM freeze on arm64 platform.
Because on arm64 platform, stop a consumer will suspend the VM,
the VM will freeze without a start consumer

Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
---
 virt/lib/irqbypass.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

Comments

Greg KH May 8, 2021, 7:22 a.m. UTC | #1
On Sat, May 08, 2021 at 03:11:52PM +0800, Zhu Lingshan wrote:
> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
> 
> The reverted commit may cause VM freeze on arm64 platform.
> Because on arm64 platform, stop a consumer will suspend the VM,
> the VM will freeze without a start consumer
> 
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> ---
>  virt/lib/irqbypass.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>
Marc Zyngier May 8, 2021, 9:29 a.m. UTC | #2
On Sat, 08 May 2021 08:11:52 +0100,
Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> 
> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
> 
> The reverted commit may cause VM freeze on arm64 platform.
> Because on arm64 platform, stop a consumer will suspend the VM,
> the VM will freeze without a start consumer

It also unconditionally calls del_consumer on the producer, which
isn't exactly expected.

> 
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

Reported-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
Suggested-by: Marc Zyngier <maz@kernel.org>
Acked-by: Marc Zyngier <maz@kernel.org>
Fixes: a979a6aa009f ("irqbypass: do not start cons/prod when failed connect")
Link: https://lore.kernel.org/r/3a2c66d6-6ca0-8478-d24b-61e8e3241b20@hisilicon.com
Cc: stable@vger.kernel.org

Thanks,

	M.
Eric Auger May 9, 2021, 5:04 p.m. UTC | #3
Hi,
On 5/8/21 11:29 AM, Marc Zyngier wrote:
> On Sat, 08 May 2021 08:11:52 +0100,
> Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>>
>> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
>>
>> The reverted commit may cause VM freeze on arm64 platform.
>> Because on arm64 platform, stop a consumer will suspend the VM,
>> the VM will freeze without a start consumer
> 
> It also unconditionally calls del_consumer on the producer, which
> isn't exactly expected.
> 
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> 
> Reported-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric

> Suggested-by: Marc Zyngier <maz@kernel.org>
> Acked-by: Marc Zyngier <maz@kernel.org>
> Fixes: a979a6aa009f ("irqbypass: do not start cons/prod when failed connect")
> Link: https://lore.kernel.org/r/3a2c66d6-6ca0-8478-d24b-61e8e3241b20@hisilicon.com
> Cc: stable@vger.kernel.org
> 
> Thanks,
> 
> 	M.
>
Shaokun Zhang May 10, 2021, 1:12 a.m. UTC | #4
Hi Marc,

On 2021/5/8 17:29, Marc Zyngier wrote:
> On Sat, 08 May 2021 08:11:52 +0100,
> Zhu Lingshan <lingshan.zhu@intel.com> wrote:
>>
>> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
>>
>> The reverted commit may cause VM freeze on arm64 platform.
>> Because on arm64 platform, stop a consumer will suspend the VM,
>> the VM will freeze without a start consumer
> 
> It also unconditionally calls del_consumer on the producer, which
> isn't exactly expected.
> 
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> 
> Reported-by: Shaokun Zhang <zhangshaokun@hisilicon.com>

Thanks for the tag, it works with this patch, So:
Tested-by: Shaokun Zhang <zhangshaokun@hisilicon.com>

I shall invite you to have a drink for the quick debug.
Anyway, thank you again.

Shaokun

> Suggested-by: Marc Zyngier <maz@kernel.org>
> Acked-by: Marc Zyngier <maz@kernel.org>
> Fixes: a979a6aa009f ("irqbypass: do not start cons/prod when failed connect")
> Link: https://lore.kernel.org/r/3a2c66d6-6ca0-8478-d24b-61e8e3241b20@hisilicon.com
> Cc: stable@vger.kernel.org
> 
> Thanks,
> 
> 	M.
>
Jason Wang May 10, 2021, 2:43 a.m. UTC | #5
在 2021/5/8 下午3:11, Zhu Lingshan 写道:
> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
>
> The reverted commit may cause VM freeze on arm64 platform.
> Because on arm64 platform, stop a consumer will suspend the VM,
> the VM will freeze without a start consumer
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>


Acked-by: Jason Wang <jasowang@redhat.com>

Please resubmit with the formal process of stable (stable-kernel-rules.rst).

Thanks


> ---
>   virt/lib/irqbypass.c | 16 ++++++----------
>   1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
> index c9bb3957f58a..28fda42e471b 100644
> --- a/virt/lib/irqbypass.c
> +++ b/virt/lib/irqbypass.c
> @@ -40,21 +40,17 @@ static int __connect(struct irq_bypass_producer *prod,
>   	if (prod->add_consumer)
>   		ret = prod->add_consumer(prod, cons);
>   
> -	if (ret)
> -		goto err_add_consumer;
> -
> -	ret = cons->add_producer(cons, prod);
> -	if (ret)
> -		goto err_add_producer;
> +	if (!ret) {
> +		ret = cons->add_producer(cons, prod);
> +		if (ret && prod->del_consumer)
> +			prod->del_consumer(prod, cons);
> +	}
>   
>   	if (cons->start)
>   		cons->start(cons);
>   	if (prod->start)
>   		prod->start(prod);
> -err_add_producer:
> -	if (prod->del_consumer)
> -		prod->del_consumer(prod, cons);
> -err_add_consumer:
> +
>   	return ret;
>   }
>
Zhu Lingshan May 10, 2021, 3 a.m. UTC | #6
On 5/10/2021 10:43 AM, Jason Wang wrote:
>
> 在 2021/5/8 下午3:11, Zhu Lingshan 写道:
>> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
>>
>> The reverted commit may cause VM freeze on arm64 platform.
>> Because on arm64 platform, stop a consumer will suspend the VM,
>> the VM will freeze without a start consumer
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>
>
> Acked-by: Jason Wang <jasowang@redhat.com>
>
> Please resubmit with the formal process of stable 
> (stable-kernel-rules.rst).
sure, I will re-submit it to stable kernel once it is merged into Linus 
tree.

Thanks
>
> Thanks
>
>
>> ---
>>   virt/lib/irqbypass.c | 16 ++++++----------
>>   1 file changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
>> index c9bb3957f58a..28fda42e471b 100644
>> --- a/virt/lib/irqbypass.c
>> +++ b/virt/lib/irqbypass.c
>> @@ -40,21 +40,17 @@ static int __connect(struct irq_bypass_producer 
>> *prod,
>>       if (prod->add_consumer)
>>           ret = prod->add_consumer(prod, cons);
>>   -    if (ret)
>> -        goto err_add_consumer;
>> -
>> -    ret = cons->add_producer(cons, prod);
>> -    if (ret)
>> -        goto err_add_producer;
>> +    if (!ret) {
>> +        ret = cons->add_producer(cons, prod);
>> +        if (ret && prod->del_consumer)
>> +            prod->del_consumer(prod, cons);
>> +    }
>>         if (cons->start)
>>           cons->start(cons);
>>       if (prod->start)
>>           prod->start(prod);
>> -err_add_producer:
>> -    if (prod->del_consumer)
>> -        prod->del_consumer(prod, cons);
>> -err_add_consumer:
>> +
>>       return ret;
>>   }
>
Jason Wang May 10, 2021, 4:34 a.m. UTC | #7
在 2021/5/10 上午11:00, Zhu, Lingshan 写道:
>
>
> On 5/10/2021 10:43 AM, Jason Wang wrote:
>>
>> 在 2021/5/8 下午3:11, Zhu Lingshan 写道:
>>> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
>>>
>>> The reverted commit may cause VM freeze on arm64 platform.
>>> Because on arm64 platform, stop a consumer will suspend the VM,
>>> the VM will freeze without a start consumer
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>
>>
>> Acked-by: Jason Wang <jasowang@redhat.com>
>>
>> Please resubmit with the formal process of stable 
>> (stable-kernel-rules.rst).
> sure, I will re-submit it to stable kernel once it is merged into 
> Linus tree.
>
> Thanks


I think it's better to resubmit (option 1), see how 
stable-kernel-rules.rst said:

""

:ref:`option_1` is **strongly** preferred, is the easiest and most common.
:ref:`option_2` and :ref:`option_3` are more useful if the patch isn't 
deemed
worthy at the time it is applied to a public git tree (for instance, because
it deserves more regression testing first).

"""

Thanks


>>
>> Thanks
>>
>>
>>> ---
>>>   virt/lib/irqbypass.c | 16 ++++++----------
>>>   1 file changed, 6 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
>>> index c9bb3957f58a..28fda42e471b 100644
>>> --- a/virt/lib/irqbypass.c
>>> +++ b/virt/lib/irqbypass.c
>>> @@ -40,21 +40,17 @@ static int __connect(struct irq_bypass_producer 
>>> *prod,
>>>       if (prod->add_consumer)
>>>           ret = prod->add_consumer(prod, cons);
>>>   -    if (ret)
>>> -        goto err_add_consumer;
>>> -
>>> -    ret = cons->add_producer(cons, prod);
>>> -    if (ret)
>>> -        goto err_add_producer;
>>> +    if (!ret) {
>>> +        ret = cons->add_producer(cons, prod);
>>> +        if (ret && prod->del_consumer)
>>> +            prod->del_consumer(prod, cons);
>>> +    }
>>>         if (cons->start)
>>>           cons->start(cons);
>>>       if (prod->start)
>>>           prod->start(prod);
>>> -err_add_producer:
>>> -    if (prod->del_consumer)
>>> -        prod->del_consumer(prod, cons);
>>> -err_add_consumer:
>>> +
>>>       return ret;
>>>   }
>>
>
Zhu Lingshan May 10, 2021, 7:09 a.m. UTC | #8
On 5/10/2021 12:34 PM, Jason Wang wrote:
>
> 在 2021/5/10 上午11:00, Zhu, Lingshan 写道:
>>
>>
>> On 5/10/2021 10:43 AM, Jason Wang wrote:
>>>
>>> 在 2021/5/8 下午3:11, Zhu Lingshan 写道:
>>>> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
>>>>
>>>> The reverted commit may cause VM freeze on arm64 platform.
>>>> Because on arm64 platform, stop a consumer will suspend the VM,
>>>> the VM will freeze without a start consumer
>>>>
>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>
>>>
>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>>
>>> Please resubmit with the formal process of stable 
>>> (stable-kernel-rules.rst).
>> sure, I will re-submit it to stable kernel once it is merged into 
>> Linus tree.
>>
>> Thanks
>
>
> I think it's better to resubmit (option 1), see how 
> stable-kernel-rules.rst said:
>
> ""
>
> :ref:`option_1` is **strongly** preferred, is the easiest and most 
> common.
> :ref:`option_2` and :ref:`option_3` are more useful if the patch isn't 
> deemed
> worthy at the time it is applied to a public git tree (for instance, 
> because
> it deserves more regression testing first).
>
> """
>
> Thanks
OK, works for me, I will add cc stable, and resubmit it soon

Thanks!
>
>
>>>
>>> Thanks
>>>
>>>
>>>> ---
>>>>   virt/lib/irqbypass.c | 16 ++++++----------
>>>>   1 file changed, 6 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
>>>> index c9bb3957f58a..28fda42e471b 100644
>>>> --- a/virt/lib/irqbypass.c
>>>> +++ b/virt/lib/irqbypass.c
>>>> @@ -40,21 +40,17 @@ static int __connect(struct irq_bypass_producer 
>>>> *prod,
>>>>       if (prod->add_consumer)
>>>>           ret = prod->add_consumer(prod, cons);
>>>>   -    if (ret)
>>>> -        goto err_add_consumer;
>>>> -
>>>> -    ret = cons->add_producer(cons, prod);
>>>> -    if (ret)
>>>> -        goto err_add_producer;
>>>> +    if (!ret) {
>>>> +        ret = cons->add_producer(cons, prod);
>>>> +        if (ret && prod->del_consumer)
>>>> +            prod->del_consumer(prod, cons);
>>>> +    }
>>>>         if (cons->start)
>>>>           cons->start(cons);
>>>>       if (prod->start)
>>>>           prod->start(prod);
>>>> -err_add_producer:
>>>> -    if (prod->del_consumer)
>>>> -        prod->del_consumer(prod, cons);
>>>> -err_add_consumer:
>>>> +
>>>>       return ret;
>>>>   }
>>>
>>
>
Marc Zyngier May 10, 2021, 7:39 a.m. UTC | #9
On Mon, 10 May 2021 02:12:22 +0100,
Shaokun Zhang <zhangshaokun@hisilicon.com> wrote:
> 
> Hi Marc,
> 
> On 2021/5/8 17:29, Marc Zyngier wrote:
> > On Sat, 08 May 2021 08:11:52 +0100,
> > Zhu Lingshan <lingshan.zhu@intel.com> wrote:
> >>
> >> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
> >>
> >> The reverted commit may cause VM freeze on arm64 platform.
> >> Because on arm64 platform, stop a consumer will suspend the VM,
> >> the VM will freeze without a start consumer
> > 
> > It also unconditionally calls del_consumer on the producer, which
> > isn't exactly expected.
> > 
> >>
> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> > 
> > Reported-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> 
> Thanks for the tag, it works with this patch, So:
> Tested-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> 
> I shall invite you to have a drink for the quick debug.
> Anyway, thank you again.

No worries, glad we were able to root-cause the problem quickly
enough.

Thanks,

	M.
Zhu Lingshan May 10, 2021, 8:32 a.m. UTC | #10
On 5/10/2021 3:09 PM, Zhu, Lingshan wrote:
>
>
> On 5/10/2021 12:34 PM, Jason Wang wrote:
>>
>> 在 2021/5/10 上午11:00, Zhu, Lingshan 写道:
>>>
>>>
>>> On 5/10/2021 10:43 AM, Jason Wang wrote:
>>>>
>>>> 在 2021/5/8 下午3:11, Zhu Lingshan 写道:
>>>>> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
>>>>>
>>>>> The reverted commit may cause VM freeze on arm64 platform.
>>>>> Because on arm64 platform, stop a consumer will suspend the VM,
>>>>> the VM will freeze without a start consumer
>>>>>
>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>>
>>>>
>>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>>>
>>>> Please resubmit with the formal process of stable 
>>>> (stable-kernel-rules.rst).
>>> sure, I will re-submit it to stable kernel once it is merged into 
>>> Linus tree.
>>>
>>> Thanks
>>
>>
>> I think it's better to resubmit (option 1), see how 
>> stable-kernel-rules.rst said:
>>
>> ""
>>
>> :ref:`option_1` is **strongly** preferred, is the easiest and most 
>> common.
>> :ref:`option_2` and :ref:`option_3` are more useful if the patch 
>> isn't deemed
>> worthy at the time it is applied to a public git tree (for instance, 
>> because
>> it deserves more regression testing first).
>>
>> """
>>
>> Thanks
> OK, works for me, I will add cc stable, and resubmit it soon
>
> Thanks!
I just seeMarc has already added "Cc: stable@vger.kernel.org", and he 
would take the patch in his tree,
so I think no need to resend.

Thanks,
Zhu Lingshan
>>
>>
>>>>
>>>> Thanks
>>>>
>>>>
>>>>> ---
>>>>>   virt/lib/irqbypass.c | 16 ++++++----------
>>>>>   1 file changed, 6 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
>>>>> index c9bb3957f58a..28fda42e471b 100644
>>>>> --- a/virt/lib/irqbypass.c
>>>>> +++ b/virt/lib/irqbypass.c
>>>>> @@ -40,21 +40,17 @@ static int __connect(struct 
>>>>> irq_bypass_producer *prod,
>>>>>       if (prod->add_consumer)
>>>>>           ret = prod->add_consumer(prod, cons);
>>>>>   -    if (ret)
>>>>> -        goto err_add_consumer;
>>>>> -
>>>>> -    ret = cons->add_producer(cons, prod);
>>>>> -    if (ret)
>>>>> -        goto err_add_producer;
>>>>> +    if (!ret) {
>>>>> +        ret = cons->add_producer(cons, prod);
>>>>> +        if (ret && prod->del_consumer)
>>>>> +            prod->del_consumer(prod, cons);
>>>>> +    }
>>>>>         if (cons->start)
>>>>>           cons->start(cons);
>>>>>       if (prod->start)
>>>>>           prod->start(prod);
>>>>> -err_add_producer:
>>>>> -    if (prod->del_consumer)
>>>>> -        prod->del_consumer(prod, cons);
>>>>> -err_add_consumer:
>>>>> +
>>>>>       return ret;
>>>>>   }
>>>>
>>>
>>
>
Marc Zyngier May 10, 2021, 10 a.m. UTC | #11
On Mon, 10 May 2021 09:32:54 +0100,
"Zhu, Lingshan" <lingshan.zhu@intel.com> wrote:
> 
> 
> 
> On 5/10/2021 3:09 PM, Zhu, Lingshan wrote:
> > 
> > 
> > On 5/10/2021 12:34 PM, Jason Wang wrote:
> >> 
> >> 在 2021/5/10 上午11:00, Zhu, Lingshan 写道:
> >>> 
> >>> 
> >>> On 5/10/2021 10:43 AM, Jason Wang wrote:
> >>>> 
> >>>> 在 2021/5/8 下午3:11, Zhu Lingshan 写道:
> >>>>> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
> >>>>> 
> >>>>> The reverted commit may cause VM freeze on arm64 platform.
> >>>>> Because on arm64 platform, stop a consumer will suspend the VM,
> >>>>> the VM will freeze without a start consumer
> >>>>> 
> >>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
> >>>> 
> >>>> 
> >>>> Acked-by: Jason Wang <jasowang@redhat.com>
> >>>> 
> >>>> Please resubmit with the formal process of stable
> >>>> (stable-kernel-rules.rst).
> >>> sure, I will re-submit it to stable kernel once it is merged into
> >>> Linus tree.
> >>> 
> >>> Thanks
> >> 
> >> 
> >> I think it's better to resubmit (option 1), see how
> >> stable-kernel-rules.rst said:
> >> 
> >> ""
> >> 
> >> :ref:`option_1` is **strongly** preferred, is the easiest and most
> >> common.
> >> :ref:`option_2` and :ref:`option_3` are more useful if the patch
> >> isn't deemed
> >> worthy at the time it is applied to a public git tree (for
> >> instance, because
> >> it deserves more regression testing first).
> >> 
> >> """
> >> 
> >> Thanks
> > OK, works for me, I will add cc stable, and resubmit it soon
> > 
> > Thanks!
> I just seeMarc has already added "Cc: stable@vger.kernel.org", and
> he would take the patch in his tree, so I think no need to resend.

That's fine, I can fix things up myself and queue the fix for -rc2.

Thanks,

	M.
Zhu Lingshan May 10, 2021, 10:37 a.m. UTC | #12
On 5/10/2021 6:00 PM, Marc Zyngier wrote:
> On Mon, 10 May 2021 09:32:54 +0100,
> "Zhu, Lingshan" <lingshan.zhu@intel.com> wrote:
>>
>>
>> On 5/10/2021 3:09 PM, Zhu, Lingshan wrote:
>>>
>>> On 5/10/2021 12:34 PM, Jason Wang wrote:
>>>> 在 2021/5/10 上午11:00, Zhu, Lingshan 写道:
>>>>>
>>>>> On 5/10/2021 10:43 AM, Jason Wang wrote:
>>>>>> 在 2021/5/8 下午3:11, Zhu Lingshan 写道:
>>>>>>> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
>>>>>>>
>>>>>>> The reverted commit may cause VM freeze on arm64 platform.
>>>>>>> Because on arm64 platform, stop a consumer will suspend the VM,
>>>>>>> the VM will freeze without a start consumer
>>>>>>>
>>>>>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>
>>>>>>
>>>>>> Acked-by: Jason Wang <jasowang@redhat.com>
>>>>>>
>>>>>> Please resubmit with the formal process of stable
>>>>>> (stable-kernel-rules.rst).
>>>>> sure, I will re-submit it to stable kernel once it is merged into
>>>>> Linus tree.
>>>>>
>>>>> Thanks
>>>>
>>>> I think it's better to resubmit (option 1), see how
>>>> stable-kernel-rules.rst said:
>>>>
>>>> ""
>>>>
>>>> :ref:`option_1` is **strongly** preferred, is the easiest and most
>>>> common.
>>>> :ref:`option_2` and :ref:`option_3` are more useful if the patch
>>>> isn't deemed
>>>> worthy at the time it is applied to a public git tree (for
>>>> instance, because
>>>> it deserves more regression testing first).
>>>>
>>>> """
>>>>
>>>> Thanks
>>> OK, works for me, I will add cc stable, and resubmit it soon
>>>
>>> Thanks!
>> I just seeMarc has already added "Cc: stable@vger.kernel.org", and
>> he would take the patch in his tree, so I think no need to resend.
> That's fine, I can fix things up myself and queue the fix for -rc2.
Thanks Marc!
>
> Thanks,
>
> 	M.
>
Marc Zyngier May 10, 2021, 11:01 a.m. UTC | #13
On Sat, 8 May 2021 15:11:52 +0800, Zhu Lingshan wrote:
> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
> 
> The reverted commit may cause VM freeze on arm64 platform.
> Because on arm64 platform, stop a consumer will suspend the VM,
> the VM will freeze without a start consumer

Applied to fixes, thanks!

[1/1] Revert "irqbypass: do not start cons/prod when failed connect"
      commit: 0b1c0157b7b0d66d2d749ec950c7f8799d4a2e0e

Cheers,

	M.
Michael S. Tsirkin May 11, 2021, 8:48 a.m. UTC | #14
On Sat, May 08, 2021 at 03:11:52PM +0800, Zhu Lingshan wrote:
> This reverts commit a979a6aa009f3c99689432e0cdb5402a4463fb88.
> 
> The reverted commit may cause VM freeze on arm64 platform.
> Because on arm64 platform, stop a consumer will suspend the VM,
> the VM will freeze without a start consumer
> 
> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  virt/lib/irqbypass.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
> index c9bb3957f58a..28fda42e471b 100644
> --- a/virt/lib/irqbypass.c
> +++ b/virt/lib/irqbypass.c
> @@ -40,21 +40,17 @@ static int __connect(struct irq_bypass_producer *prod,
>  	if (prod->add_consumer)
>  		ret = prod->add_consumer(prod, cons);
>  
> -	if (ret)
> -		goto err_add_consumer;
> -
> -	ret = cons->add_producer(cons, prod);
> -	if (ret)
> -		goto err_add_producer;
> +	if (!ret) {
> +		ret = cons->add_producer(cons, prod);
> +		if (ret && prod->del_consumer)
> +			prod->del_consumer(prod, cons);
> +	}
>  
>  	if (cons->start)
>  		cons->start(cons);
>  	if (prod->start)
>  		prod->start(prod);
> -err_add_producer:
> -	if (prod->del_consumer)
> -		prod->del_consumer(prod, cons);
> -err_add_consumer:
> +
>  	return ret;
>  }
>  
> -- 
> 2.27.0
diff mbox series

Patch

diff --git a/virt/lib/irqbypass.c b/virt/lib/irqbypass.c
index c9bb3957f58a..28fda42e471b 100644
--- a/virt/lib/irqbypass.c
+++ b/virt/lib/irqbypass.c
@@ -40,21 +40,17 @@  static int __connect(struct irq_bypass_producer *prod,
 	if (prod->add_consumer)
 		ret = prod->add_consumer(prod, cons);
 
-	if (ret)
-		goto err_add_consumer;
-
-	ret = cons->add_producer(cons, prod);
-	if (ret)
-		goto err_add_producer;
+	if (!ret) {
+		ret = cons->add_producer(cons, prod);
+		if (ret && prod->del_consumer)
+			prod->del_consumer(prod, cons);
+	}
 
 	if (cons->start)
 		cons->start(cons);
 	if (prod->start)
 		prod->start(prod);
-err_add_producer:
-	if (prod->del_consumer)
-		prod->del_consumer(prod, cons);
-err_add_consumer:
+
 	return ret;
 }