Messages in this thread | | | Subject | Re: [PATCH v2 1/3] genirq/affinity: Add irq_update_affinity_desc() | From | John Garry <> | Date | Mon, 23 Nov 2020 12:54:45 +0000 |
| |
Hi Marc,
>>> Right, but if the driver is removed then the interrupts should be >>> deallocated, right? >>> >> >> When removing the driver we just call free_irq(), which removes the >> handler and disables the interrupt. >> >> But about the irq_desc, this is created when the mapping is created in >> irq_create_fwspec_mapping(), and I don't see this being torn down in >> the driver removal, so persistent in that regard. > > If the irq_descs are created via the platform_get_irq() calls in > platform_get_irqs_affinity(), I'd expect some equivalent helper to > tear things down as a result, calling irq_dispose_mapping() behind the > scenes. >
So this looks lacking in the kernel AFAICS...
So is there a reason for which irq dispose mapping is not a requirement for drivers when finished with the irq? because of shared interrupts?
>> So for pci msi I can see that we free the irq_desc in >> pci_disable_msi() -> free_msi_irqs() -> msi_domain_free_irqs() ... >> >> So what I am missing here? > > I'm not sure the paths are strictly equivalent. On the PCI side, we > can have something that completely driver agnostic, as it is all > architectural. In your case, only the endpoint driver knows about what > happens, and needs to free things accordingly. > > Finally, there is the issue in your driver that everything is > requested using devm_request_irq, which cannot play nicely with an > explicit irq_desc teardown. You'll probably need to provide the > equivalent devm helpers for your driver to safely be taken down. >
Yeah, so since we use the devm irq request method, we also need a devm dispose release method as we can't dispose the irq mapping in the remove() method, prior to the irq_free() in the later devm release method.
But it looks like there is more to it than that, which I'm worried is far from non-trivial. For example, just calling irq_dispose_mapping() for removal and then plaform_get_irq()->acpi_get_irq() second time fails as it looks like more tidy-up is needed for removal...
Cheers, John
| |