| Message ID | 60028de8-a137-423d-91d8-00b2942bd73d@moroto.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/events: fix error codes in xen_bind_pirq_msi_to_irq() | expand |
On 27.11.23 13:57, Dan Carpenter wrote: > The error code needs to be set on these error paths. > > Fixes: 5dd9ad32d775 ("xen/events: drop xen_allocate_irqs_dynamic()") > Fixes: d2ba3166f23b ("xen/events: move drivers/xen/events.c into drivers/xen/events/") Please drop the last Fixes: tag. Said patch didn't introduce any new problem. > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > Are we going to backport these to stable? Should I split this into two > patches? > > drivers/xen/events/events_base.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c > index f5edb9e27e3c..aae62603b461 100644 > --- a/drivers/xen/events/events_base.c > +++ b/drivers/xen/events/events_base.c > @@ -1105,13 +1105,17 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc, > mutex_lock(&irq_mapping_update_lock); > > irq = irq_alloc_descs(-1, 0, nvec, -1); > - if (irq < 0) > + if (irq < 0) { > + ret = irq; > goto out; > + } Why? The return value for the out: label is in irq. > > for (i = 0; i < nvec; i++) { > info = xen_irq_init(irq + i); > - if (!info) > + if (!info) { > + ret = -ENOMEM; > goto error_irq; > + } It would be easier to just preset ret with -ENOMEM when defining it. Juergen
On Mon, Nov 27, 2023 at 02:17:05PM +0100, Juergen Gross wrote: > On 27.11.23 13:57, Dan Carpenter wrote: > > The error code needs to be set on these error paths. > > > > Fixes: 5dd9ad32d775 ("xen/events: drop xen_allocate_irqs_dynamic()") > > Fixes: d2ba3166f23b ("xen/events: move drivers/xen/events.c into drivers/xen/events/") > > Please drop the last Fixes: tag. Said patch didn't introduce any new problem. Yup. > > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > > --- > > Are we going to backport these to stable? Should I split this into two > > patches? > > > > drivers/xen/events/events_base.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c > > index f5edb9e27e3c..aae62603b461 100644 > > --- a/drivers/xen/events/events_base.c > > +++ b/drivers/xen/events/events_base.c > > @@ -1105,13 +1105,17 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc, > > mutex_lock(&irq_mapping_update_lock); > > irq = irq_alloc_descs(-1, 0, nvec, -1); > > - if (irq < 0) > > + if (irq < 0) { > > + ret = irq; > > goto out; > > + } > > Why? The return value for the out: label is in irq. > This patch is full of embarrassment. I misread this code. I thought the out label was in the error handling block. > > for (i = 0; i < nvec; i++) { > > info = xen_irq_init(irq + i); > > - if (!info) > > + if (!info) { > > + ret = -ENOMEM; > > goto error_irq; > > + } > > It would be easier to just preset ret with -ENOMEM when defining it. > That only works if it fails on the first iteration. I'll fix this up and resend. regards, dan carpenter
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c index f5edb9e27e3c..aae62603b461 100644 --- a/drivers/xen/events/events_base.c +++ b/drivers/xen/events/events_base.c @@ -1105,13 +1105,17 @@ int xen_bind_pirq_msi_to_irq(struct pci_dev *dev, struct msi_desc *msidesc, mutex_lock(&irq_mapping_update_lock); irq = irq_alloc_descs(-1, 0, nvec, -1); - if (irq < 0) + if (irq < 0) { + ret = irq; goto out; + } for (i = 0; i < nvec; i++) { info = xen_irq_init(irq + i); - if (!info) + if (!info) { + ret = -ENOMEM; goto error_irq; + } irq_set_chip_and_handler_name(irq + i, &xen_pirq_chip, handle_edge_irq, name);
The error code needs to be set on these error paths. Fixes: 5dd9ad32d775 ("xen/events: drop xen_allocate_irqs_dynamic()") Fixes: d2ba3166f23b ("xen/events: move drivers/xen/events.c into drivers/xen/events/") Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> --- Are we going to backport these to stable? Should I split this into two patches? drivers/xen/events/events_base.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)