Messages in this thread Patch in this message | | | From | "Shah, Amit" <> | Subject | Re: [PATCH v2 2/2] xen: events: free irqs in error condition | Date | Tue, 27 Feb 2018 17:32:53 +0000 |
| |
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
| |