Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH] PCI: altera-msi: Remove irq handler and data in one go | Date | Thu, 12 Nov 2020 12:28:53 +0100 |
| |
On Wed, Nov 11 2020 at 16:16, Bjorn Helgaas wrote: > On Wed, Nov 11, 2020 at 10:43:55PM +0100, Martin Kaiser wrote: >> This function uses the error status from irq_set_handler_data(). >> irq_set_chained_handler_and_data() returns no such error status. Is it >> ok to drop the error handling? > > I'm not an IRQ expert, but I'd say it's OK to drop it. Of the 40 or > so callers, the only other caller that looks at the error status is > ingenic_intc_of_init().
Don't know why irq_set_chained_handler_and_data() does not return an error, but the call site must really do something stupid if it fails to hand in the proper interrupt number.
> Thomas, it looks like irq_domain_set_info() and msi_domain_ops_init() > set the handler itself before setting the handler data: > > irq_domain_set_info > irq_set_chip_and_handler_name(virq, chip, handler, ...) > irq_set_handler_data(virq, handler_data) > > msi_domain_ops_init > __irq_set_handler(virq, info->handler, ...) > if (info->handler_data) > irq_set_handler_data(virq, info->handler_data) > > That looks at least superficially similar to the race you fixed with > 2cf5a03cb29d ("PCI/keystone: Fix race in installing chained IRQ > handler"). > > Should irq_domain_set_info() and msi_domain_ops_init() swap the order, > too?
In theory yes. Practically it should not matter because that happens during the allocation way before the interrupt can actually fire. I'll have a deeper look nevertheless.
Thanks,
tglx
| |