* [PATCH v2 0/2] xen: fix bugs in error conditions
@ 2018-02-27 15:55 Amit Shah
2018-02-27 15:55 ` [PATCH v2 1/2] xen: fix out-of-bounds irq unbind for MSI message groups Amit Shah
2018-02-27 15:55 ` [PATCH v2 2/2] xen: events: free irqs in error condition Amit Shah
0 siblings, 2 replies; 9+ messages in thread
From: Amit Shah @ 2018-02-27 15:55 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, Amit Shah, Roger Pau Monné,
Boris Ostrovsky, Eduardo Valentin, Juergen Gross,
Thomas Gleixner, K. Y. Srinivasan, Liu Shuo, Anoob Soman
Hello,
These bugs were found during code review. Details in the commits.
Please review and apply.
v2:
- fix up patch 2 properly (Roger Pau Monné)
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Eduardo Valentin <eduval@amazon.com>
CC: Juergen Gross <jgross@suse.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Liu Shuo <shuo.a.liu@intel.com>
CC: Anoob Soman <anoob.soman@citrix.com>
Amit Shah (2):
xen: fix out-of-bounds irq unbind for MSI message groups
xen: events: free irqs in error condition
drivers/xen/events/events_base.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
--
2.7.3.AMZN
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/2] xen: fix out-of-bounds irq unbind for MSI message groups
2018-02-27 15:55 [PATCH v2 0/2] xen: fix bugs in error conditions Amit Shah
@ 2018-02-27 15:55 ` Amit Shah
2018-02-27 16:58 ` Roger Pau Monné
2018-02-27 15:55 ` [PATCH v2 2/2] xen: events: free irqs in error condition Amit Shah
1 sibling, 1 reply; 9+ messages in thread
From: Amit Shah @ 2018-02-27 15:55 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, Amit Shah, stable, Roger Pau Monné,
Boris Ostrovsky, Eduardo Valentin, Juergen Gross,
Thomas Gleixner, K. Y. Srinivasan, Liu Shuo, Anoob Soman
When an MSI descriptor was not available, the error path would try to
unbind an irq that was never acquired - potentially unbinding an
unrelated irq.
Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
Reported-by: Hooman Mirhadi <mirhadih@amazon.com>
CC: <stable@vger.kernel.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Eduardo Valentin <eduval@amazon.com>
CC: Juergen Gross <jgross@suse.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Liu Shuo <shuo.a.liu@intel.com>
CC: Anoob Soman <anoob.soman@citrix.com>
Signed-off-by: Amit Shah <aams@amazon.com>
---
drivers/xen/events/events_base.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 1ab4bd1..c86d10e 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -755,8 +755,10 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
mutex_unlock(&irq_mapping_update_lock);
return irq;
error_irq:
- for (; i >= 0; i--)
+ while (i > 0) {
+ i--;
__unbind_from_irq(irq + i);
+ }
mutex_unlock(&irq_mapping_update_lock);
return ret;
}
--
2.7.3.AMZN
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] xen: events: free irqs in error condition
2018-02-27 15:55 [PATCH v2 0/2] xen: fix bugs in error conditions Amit Shah
2018-02-27 15:55 ` [PATCH v2 1/2] xen: fix out-of-bounds irq unbind for MSI message groups Amit Shah
@ 2018-02-27 15:55 ` Amit Shah
2018-02-27 17:07 ` Roger Pau Monné
1 sibling, 1 reply; 9+ messages in thread
From: Amit Shah @ 2018-02-27 15:55 UTC (permalink / raw)
To: xen-devel
Cc: linux-kernel, Amit Shah, stable, Roger Pau Monné,
Boris Ostrovsky, Eduardo Valentin, Juergen Gross,
Thomas Gleixner, K. Y. Srinivasan, Liu Shuo, Anoob Soman
In case of errors in irq setup for MSI, free up the allocated irqs.
Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
Reported-by: Hooman Mirhadi <mirhadih@amazon.com>
CC: <stable@vger.kernel.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Eduardo Valentin <eduval@amazon.com>
CC: Juergen Gross <jgross@suse.com>
CC: Thomas Gleixner <tglx@linutronix.de>
CC: "K. Y. Srinivasan" <kys@microsoft.com>
CC: Liu Shuo <shuo.a.liu@intel.com>
CC: Anoob Soman <anoob.soman@citrix.com>
Signed-off-by: Amit Shah <aams@amazon.com>
---
drivers/xen/events/events_base.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index c86d10e..a299586 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -750,11 +750,14 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
ret = irq_set_msi_desc(irq, msidesc);
if (ret < 0)
- goto error_irq;
+ goto error_desc;
out:
mutex_unlock(&irq_mapping_update_lock);
return irq;
error_irq:
+ while (--nvec >= i)
+ xen_free_irq(irq + nvec);
+error_desc:
while (i > 0) {
i--;
__unbind_from_irq(irq + i);
--
2.7.3.AMZN
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] xen: fix out-of-bounds irq unbind for MSI message groups
2018-02-27 15:55 ` [PATCH v2 1/2] xen: fix out-of-bounds irq unbind for MSI message groups Amit Shah
@ 2018-02-27 16:58 ` Roger Pau Monné
0 siblings, 0 replies; 9+ messages in thread
From: Roger Pau Monné @ 2018-02-27 16:58 UTC (permalink / raw)
To: Amit Shah
Cc: xen-devel, linux-kernel, stable, Boris Ostrovsky,
Eduardo Valentin, Juergen Gross, Thomas Gleixner,
K. Y. Srinivasan, Liu Shuo, Anoob Soman
On Tue, Feb 27, 2018 at 03:55:57PM +0000, Amit Shah wrote:
> When an MSI descriptor was not available, the error path would try to
> unbind an irq that was never acquired - potentially unbinding an
> unrelated irq.
Those IRQs have been allocated in the xen_allocate_irqs_dynamic call,
so I think the "potentially unbinding an unrelated irq" part is wrong.
The unbind call would be performed against an unbound IRQ, which is
harmless AFAICT.
> Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
> Reported-by: Hooman Mirhadi <mirhadih@amazon.com>
> CC: <stable@vger.kernel.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Eduardo Valentin <eduval@amazon.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> CC: Liu Shuo <shuo.a.liu@intel.com>
> CC: Anoob Soman <anoob.soman@citrix.com>
> Signed-off-by: Amit Shah <aams@amazon.com>
> ---
> drivers/xen/events/events_base.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 1ab4bd1..c86d10e 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -755,8 +755,10 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
> mutex_unlock(&irq_mapping_update_lock);
> return irq;
> error_irq:
> - for (; i >= 0; i--)
> + while (i > 0) {
while (i--)
__unbind_from_irq(irq + i);
Although please see reply to patch 2.
Roger.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] xen: events: free irqs in error condition
2018-02-27 15:55 ` [PATCH v2 2/2] xen: events: free irqs in error condition Amit Shah
@ 2018-02-27 17:07 ` Roger Pau Monné
2018-02-27 17:32 ` Shah, Amit
0 siblings, 1 reply; 9+ messages in thread
From: Roger Pau Monné @ 2018-02-27 17:07 UTC (permalink / raw)
To: Amit Shah
Cc: xen-devel, linux-kernel, stable, Boris Ostrovsky,
Eduardo Valentin, Juergen Gross, Thomas Gleixner,
K. Y. Srinivasan, Liu Shuo, Anoob Soman
On Tue, Feb 27, 2018 at 03:55:58PM +0000, Amit Shah wrote:
> In case of errors in irq setup for MSI, free up the allocated irqs.
>
> Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
> Reported-by: Hooman Mirhadi <mirhadih@amazon.com>
> CC: <stable@vger.kernel.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Eduardo Valentin <eduval@amazon.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: Thomas Gleixner <tglx@linutronix.de>
> CC: "K. Y. Srinivasan" <kys@microsoft.com>
> CC: Liu Shuo <shuo.a.liu@intel.com>
> CC: Anoob Soman <anoob.soman@citrix.com>
> Signed-off-by: Amit Shah <aams@amazon.com>
> ---
> drivers/xen/events/events_base.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index c86d10e..a299586 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -750,11 +750,14 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc,
>
> ret = irq_set_msi_desc(irq, msidesc);
> if (ret < 0)
> - goto error_irq;
> + goto error_desc;
> out:
> mutex_unlock(&irq_mapping_update_lock);
> return irq;
> error_irq:
> + while (--nvec >= i)
> + xen_free_irq(irq + nvec);
> +error_desc:
> while (i > 0) {
> i--;
> __unbind_from_irq(irq + i);
It seems pointless to introduce another label and another loop to fix
something that can be fixed with a single label and a single loop,
this just makes the code more complex for no reason.
IMHO the way to solve this issue is:
while (nvec--) {
if (nvec >= i)
xen_free_irq(irq + nvec);
else
__unbind_from_irq(irq + nvec);
}
Roger.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] xen: events: free irqs in error condition
2018-02-27 17:07 ` Roger Pau Monné
@ 2018-02-27 17:32 ` Shah, Amit
2018-02-28 8:16 ` Roger Pau Monné
0 siblings, 1 reply; 9+ messages in thread
From: Shah, Amit @ 2018-02-27 17:32 UTC (permalink / raw)
To: roger.pau
Cc: boris.ostrovsky, linux-kernel, tglx, kys, Valentin, Eduardo,
jgross, stable, shuo.a.liu, anoob.soman, xen-devel
On Di, 2018-02-27 at 17:07 +0000, Roger Pau Monné wrote:
> On Tue, Feb 27, 2018 at 03:55:58PM +0000, Amit Shah wrote:
> >
> > In case of errors in irq setup for MSI, free up the allocated irqs.
> >
> > Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
> > Reported-by: Hooman Mirhadi <mirhadih@amazon.com>
> > CC: <stable@vger.kernel.org>
> > CC: Roger Pau Monné <roger.pau@citrix.com>
> > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > CC: Eduardo Valentin <eduval@amazon.com>
> > CC: Juergen Gross <jgross@suse.com>
> > CC: Thomas Gleixner <tglx@linutronix.de>
> > CC: "K. Y. Srinivasan" <kys@microsoft.com>
> > CC: Liu Shuo <shuo.a.liu@intel.com>
> > CC: Anoob Soman <anoob.soman@citrix.com>
> > Signed-off-by: Amit Shah <aams@amazon.com>
> > ---
> > drivers/xen/events/events_base.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/xen/events/events_base.c
> > b/drivers/xen/events/events_base.c
> > index c86d10e..a299586 100644
> > --- a/drivers/xen/events/events_base.c
> > +++ b/drivers/xen/events/events_base.c
> > @@ -750,11 +750,14 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev
> > *dev, struct msi_desc *msidesc,
> >
> > ret = irq_set_msi_desc(irq, msidesc);
> > if (ret < 0)
> > - goto error_irq;
> > + goto error_desc;
> > out:
> > mutex_unlock(&irq_mapping_update_lock);
> > return irq;
> > error_irq:
> > + while (--nvec >= i)
> > + xen_free_irq(irq + nvec);
> > +error_desc:
> > while (i > 0) {
> > i--;
> > __unbind_from_irq(irq + i);
> It seems pointless to introduce another label and another loop to fix
> something that can be fixed with a single label and a single loop,
> this just makes the code more complex for no reason.
I disagree, just because there are two different cleanups to be made
for two different issues; it's not as if the if.. and else conditions
are going to be interleaved.
Anyway it's a matter of taste.
Since you've already proposed the patch, would you mind baking a proper
one and posting it?
Thanks!
> IMHO the way to solve this issue is:
>
> while (nvec--) {
> if (nvec >= i)
> xen_free_irq(irq + nvec);
> else
> __unbind_from_irq(irq + nvec);
> }
Amit
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] xen: events: free irqs in error condition
2018-02-27 17:32 ` Shah, Amit
@ 2018-02-28 8:16 ` Roger Pau Monné
2018-02-28 8:25 ` Shah, Amit
0 siblings, 1 reply; 9+ messages in thread
From: Roger Pau Monné @ 2018-02-28 8:16 UTC (permalink / raw)
To: Shah, Amit
Cc: boris.ostrovsky, linux-kernel, tglx, kys, Valentin, Eduardo,
jgross, stable, shuo.a.liu, anoob.soman, xen-devel
On Tue, Feb 27, 2018 at 05:32:53PM +0000, Shah, Amit wrote:
>
> On Di, 2018-02-27 at 17:07 +0000, Roger Pau Monné wrote:
> > On Tue, Feb 27, 2018 at 03:55:58PM +0000, Amit Shah wrote:
> > >
> > > In case of errors in irq setup for MSI, free up the allocated irqs.
> > >
> > > Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message groups")
> > > Reported-by: Hooman Mirhadi <mirhadih@amazon.com>
> > > CC: <stable@vger.kernel.org>
> > > CC: Roger Pau Monné <roger.pau@citrix.com>
> > > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > > CC: Eduardo Valentin <eduval@amazon.com>
> > > CC: Juergen Gross <jgross@suse.com>
> > > CC: Thomas Gleixner <tglx@linutronix.de>
> > > CC: "K. Y. Srinivasan" <kys@microsoft.com>
> > > CC: Liu Shuo <shuo.a.liu@intel.com>
> > > CC: Anoob Soman <anoob.soman@citrix.com>
> > > Signed-off-by: Amit Shah <aams@amazon.com>
> > > ---
> > > drivers/xen/events/events_base.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/xen/events/events_base.c
> > > b/drivers/xen/events/events_base.c
> > > index c86d10e..a299586 100644
> > > --- a/drivers/xen/events/events_base.c
> > > +++ b/drivers/xen/events/events_base.c
> > > @@ -750,11 +750,14 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev
> > > *dev, struct msi_desc *msidesc,
> > >
> > > ret = irq_set_msi_desc(irq, msidesc);
> > > if (ret < 0)
> > > - goto error_irq;
> > > + goto error_desc;
> > > out:
> > > mutex_unlock(&irq_mapping_update_lock);
> > > return irq;
> > > error_irq:
> > > + while (--nvec >= i)
> > > + xen_free_irq(irq + nvec);
> > > +error_desc:
> > > while (i > 0) {
> > > i--;
> > > __unbind_from_irq(irq + i);
> > It seems pointless to introduce another label and another loop to fix
> > something that can be fixed with a single label and a single loop,
> > this just makes the code more complex for no reason.
>
> I disagree, just because there are two different cleanups to be made
> for two different issues; it's not as if the if.. and else conditions
> are going to be interleaved.
Oh, I don't mind so much whether it ends up being two patches or a
single one, but IMHO the code should end up looking similar to what I
proposed, I would like to avoid having two loops and two labels.
Could you rework the series so that the end result uses a single loop
(and label)?
Thanks, Roger.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] xen: events: free irqs in error condition
2018-02-28 8:16 ` Roger Pau Monné
@ 2018-02-28 8:25 ` Shah, Amit
2018-02-28 8:27 ` Juergen Gross
0 siblings, 1 reply; 9+ messages in thread
From: Shah, Amit @ 2018-02-28 8:25 UTC (permalink / raw)
To: roger.pau
Cc: boris.ostrovsky, linux-kernel, tglx, kys, jgross, stable,
shuo.a.liu, anoob.soman, Valentin, Eduardo, xen-devel
On Mi, 2018-02-28 at 08:16 +0000, Roger Pau Monné wrote:
> On Tue, Feb 27, 2018 at 05:32:53PM +0000, Shah, Amit wrote:
> >
> >
> > On Di, 2018-02-27 at 17:07 +0000, Roger Pau Monné wrote:
> > >
> > > On Tue, Feb 27, 2018 at 03:55:58PM +0000, Amit Shah wrote:
> > > >
> > > >
> > > > In case of errors in irq setup for MSI, free up the allocated
> > > > irqs.
> > > >
> > > > Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message
> > > > groups")
> > > > Reported-by: Hooman Mirhadi <mirhadih@amazon.com>
> > > > CC: <stable@vger.kernel.org>
> > > > CC: Roger Pau Monné <roger.pau@citrix.com>
> > > > CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > > > CC: Eduardo Valentin <eduval@amazon.com>
> > > > CC: Juergen Gross <jgross@suse.com>
> > > > CC: Thomas Gleixner <tglx@linutronix.de>
> > > > CC: "K. Y. Srinivasan" <kys@microsoft.com>
> > > > CC: Liu Shuo <shuo.a.liu@intel.com>
> > > > CC: Anoob Soman <anoob.soman@citrix.com>
> > > > Signed-off-by: Amit Shah <aams@amazon.com>
> > > > ---
> > > > drivers/xen/events/events_base.c | 5 ++++-
> > > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/xen/events/events_base.c
> > > > b/drivers/xen/events/events_base.c
> > > > index c86d10e..a299586 100644
> > > > --- a/drivers/xen/events/events_base.c
> > > > +++ b/drivers/xen/events/events_base.c
> > > > @@ -750,11 +750,14 @@ int xen_bind_pirq_msi_to_irq(struct
> > > > pci_dev
> > > > *dev, struct msi_desc *msidesc,
> > > >
> > > > ret = irq_set_msi_desc(irq, msidesc);
> > > > if (ret < 0)
> > > > - goto error_irq;
> > > > + goto error_desc;
> > > > out:
> > > > mutex_unlock(&irq_mapping_update_lock);
> > > > return irq;
> > > > error_irq:
> > > > + while (--nvec >= i)
> > > > + xen_free_irq(irq + nvec);
> > > > +error_desc:
> > > > while (i > 0) {
> > > > i--;
> > > > __unbind_from_irq(irq + i);
> > > It seems pointless to introduce another label and another loop to
> > > fix
> > > something that can be fixed with a single label and a single
> > > loop,
> > > this just makes the code more complex for no reason.
> > I disagree, just because there are two different cleanups to be
> > made
> > for two different issues; it's not as if the if.. and else
> > conditions
> > are going to be interleaved.
> Oh, I don't mind so much whether it ends up being two patches or a
> single one, but IMHO the code should end up looking similar to what I
> proposed, I would like to avoid having two loops and two labels.
>
> Could you rework the series so that the end result uses a single loop
> (and label)?
That was the part I didn't like much, so it would be better if the
patch came from you :)
Amit
Amazon Development Center Germany GmbH
Berlin - Dresden - Aachen
main office: Krausenstr. 38, 10117 Berlin
Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger
Ust-ID: DE289237879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] xen: events: free irqs in error condition
2018-02-28 8:25 ` Shah, Amit
@ 2018-02-28 8:27 ` Juergen Gross
0 siblings, 0 replies; 9+ messages in thread
From: Juergen Gross @ 2018-02-28 8:27 UTC (permalink / raw)
To: Shah, Amit, roger.pau
Cc: boris.ostrovsky, linux-kernel, tglx, kys, stable, shuo.a.liu,
anoob.soman, Valentin, Eduardo, xen-devel
On 28/02/18 09:25, Shah, Amit wrote:
>
> On Mi, 2018-02-28 at 08:16 +0000, Roger Pau Monné wrote:
>> On Tue, Feb 27, 2018 at 05:32:53PM +0000, Shah, Amit wrote:
>>>
>>>
>>> On Di, 2018-02-27 at 17:07 +0000, Roger Pau Monné wrote:
>>>>
>>>> On Tue, Feb 27, 2018 at 03:55:58PM +0000, Amit Shah wrote:
>>>>>
>>>>>
>>>>> In case of errors in irq setup for MSI, free up the allocated
>>>>> irqs.
>>>>>
>>>>> Fixes: 4892c9b4ada9f9 ("xen: add support for MSI message
>>>>> groups")
>>>>> Reported-by: Hooman Mirhadi <mirhadih@amazon.com>
>>>>> CC: <stable@vger.kernel.org>
>>>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>>>> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>>>>> CC: Eduardo Valentin <eduval@amazon.com>
>>>>> CC: Juergen Gross <jgross@suse.com>
>>>>> CC: Thomas Gleixner <tglx@linutronix.de>
>>>>> CC: "K. Y. Srinivasan" <kys@microsoft.com>
>>>>> CC: Liu Shuo <shuo.a.liu@intel.com>
>>>>> CC: Anoob Soman <anoob.soman@citrix.com>
>>>>> Signed-off-by: Amit Shah <aams@amazon.com>
>>>>> ---
>>>>> drivers/xen/events/events_base.c | 5 ++++-
>>>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/xen/events/events_base.c
>>>>> b/drivers/xen/events/events_base.c
>>>>> index c86d10e..a299586 100644
>>>>> --- a/drivers/xen/events/events_base.c
>>>>> +++ b/drivers/xen/events/events_base.c
>>>>> @@ -750,11 +750,14 @@ int xen_bind_pirq_msi_to_irq(struct
>>>>> pci_dev
>>>>> *dev, struct msi_desc *msidesc,
>>>>>
>>>>> ret = irq_set_msi_desc(irq, msidesc);
>>>>> if (ret < 0)
>>>>> - goto error_irq;
>>>>> + goto error_desc;
>>>>> out:
>>>>> mutex_unlock(&irq_mapping_update_lock);
>>>>> return irq;
>>>>> error_irq:
>>>>> + while (--nvec >= i)
>>>>> + xen_free_irq(irq + nvec);
>>>>> +error_desc:
>>>>> while (i > 0) {
>>>>> i--;
>>>>> __unbind_from_irq(irq + i);
>>>> It seems pointless to introduce another label and another loop to
>>>> fix
>>>> something that can be fixed with a single label and a single
>>>> loop,
>>>> this just makes the code more complex for no reason.
>>> I disagree, just because there are two different cleanups to be
>>> made
>>> for two different issues; it's not as if the if.. and else
>>> conditions
>>> are going to be interleaved.
>> Oh, I don't mind so much whether it ends up being two patches or a
>> single one, but IMHO the code should end up looking similar to what I
>> proposed, I would like to avoid having two loops and two labels.
>>
>> Could you rework the series so that the end result uses a single loop
>> (and label)?
>
> That was the part I didn't like much, so it would be better if the
> patch came from you :)
I'd prefer Roger's solution, too.
Roger, in case you don't want to write the patch, I can do it.
Juergen
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-02-28 8:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-27 15:55 [PATCH v2 0/2] xen: fix bugs in error conditions Amit Shah
2018-02-27 15:55 ` [PATCH v2 1/2] xen: fix out-of-bounds irq unbind for MSI message groups Amit Shah
2018-02-27 16:58 ` Roger Pau Monné
2018-02-27 15:55 ` [PATCH v2 2/2] xen: events: free irqs in error condition Amit Shah
2018-02-27 17:07 ` Roger Pau Monné
2018-02-27 17:32 ` Shah, Amit
2018-02-28 8:16 ` Roger Pau Monné
2018-02-28 8:25 ` Shah, Amit
2018-02-28 8:27 ` Juergen Gross
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).