| Message ID | 20210508071152.722425-1-lingshan.zhu@intel.com |
|---|---|
| State | Accepted |
| Commit | e44b49f623c77bee7451f1a82ccfb969c1028ae2 |
| Headers | show |
| Series | Revert "irqbypass: do not start cons/prod when failed connect" | expand |
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. >
在 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; > } >
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; >> } >
在 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; >>> } >> >
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; >>>> } >>> >> >
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. -- Without deviation from the norm, progress is not possible.
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; >>>>> } >>>> >>> >> >
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. -- Without deviation from the norm, progress is not possible.
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. >
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. -- Without deviation from the norm, progress is not possible.
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 --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; }
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(-)