Parallel probing (e.g. due to asynchronous probing) of devices that
share interrupts can currently result in two mappings for the same
hardware interrupt to be created.
This series fixes this mapping race and clean up the irqdomain locking
so that in the end the global irq_domain_mutex is only used for managing
the likewise global irq_domain_list, while domain operations (e.g.
IRQ allocations) use per-domain (hierarchy) locking.
Johan
Changes in v2
- split out redundant-lookup cleanup (1/4)
- use a per-domain mutex to address mapping race (2/4)
- move kernel-doc to exported function (2/4)
- fix association race (3/4, new)
- use per-domain mutex for associations (4/4, new)
Changes in v3
- drop dead and bogus code (1--3/19, new)
- fix racy mapcount accesses (5/19, new)
- drop revmap mutex (6/19, new)
- use irq_domain_mutex to address mapping race (9/19)
- clean up irq_domain_push/pop_irq() (10/19, new)
- use irq_domain_create_hierarchy() to construct hierarchies
(11--18/19, new)
- switch to per-domain locking (19/19, new)
Johan Hovold (19):
irqdomain: Drop bogus fwspec-mapping error handling
irqdomain: Drop dead domain-name assignment
irqdomain: Drop leftover brackets
irqdomain: Fix association race
irqdomain: Fix disassociation race
irqdomain: Drop revmap mutex
irqdomain: Look for existing mapping only once
irqdomain: Refactor __irq_domain_alloc_irqs()
irqdomain: Fix mapping-creation race
irqdomain: Clean up irq_domain_push/pop_irq()
x86/ioapic: Use irq_domain_create_hierarchy()
x86/apic: Use irq_domain_create_hierarchy()
irqchip/alpine-msi: Use irq_domain_add_hierarchy()
irqchip/gic-v2m: Use irq_domain_create_hierarchy()
irqchip/gic-v3-its: Use irq_domain_create_hierarchy()
irqchip/gic-v3-mbi: Use irq_domain_create_hierarchy()
irqchip/loongson-pch-msi: Use irq_domain_create_hierarchy()
irqchip/mvebu-odmi: Use irq_domain_create_hierarchy()
irqdomain: Switch to per-domain locking
arch/x86/kernel/apic/io_apic.c | 8 +-
arch/x86/platform/uv/uv_irq.c | 7 +-
drivers/irqchip/irq-alpine-msi.c | 8 +-
drivers/irqchip/irq-gic-v2m.c | 5 +-
drivers/irqchip/irq-gic-v3-its.c | 13 +-
drivers/irqchip/irq-gic-v3-mbi.c | 5 +-
drivers/irqchip/irq-loongson-pch-msi.c | 9 +-
drivers/irqchip/irq-mvebu-odmi.c | 13 +-
include/linux/irqdomain.h | 6 +-
kernel/irq/irqdomain.c | 328 ++++++++++++++-----------
10 files changed, 220 insertions(+), 182 deletions(-)
--
2.37.4
Parallel probing (e.g. due to asynchronous probing) of devices that share
interrupts can currently result in two mappings for the same hardware
interrupt to be created.
Make sure to hold the irq_domain_mutex when creating mappings so that
looking for an existing mapping before creating a new one is done
atomically.
Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers")
Fixes: b62b2cf5759b ("irqdomain: Fix handling of type settings for existing mappings")
Cc: Dmitry Torokhov <[email protected]>
Cc: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Johan Hovold <[email protected]>
---
kernel/irq/irqdomain.c | 47 ++++++++++++++++++++++++++++++++----------
1 file changed, 36 insertions(+), 11 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index d6139b0218d4..7232947eee3e 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -25,6 +25,9 @@ static DEFINE_MUTEX(irq_domain_mutex);
static struct irq_domain *irq_default_domain;
+static int ___irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
+ unsigned int nr_irqs, int node, void *arg,
+ bool realloc, const struct irq_affinity_desc *affinity);
static void irq_domain_check_hierarchy(struct irq_domain *domain);
struct irqchip_fwid {
@@ -692,7 +695,7 @@ static unsigned int __irq_create_mapping_affinity(struct irq_domain *domain,
return 0;
}
- if (irq_domain_associate(domain, virq, hwirq)) {
+ if (__irq_domain_associate(domain, virq, hwirq)) {
irq_free_desc(virq);
return 0;
}
@@ -728,14 +731,20 @@ unsigned int irq_create_mapping_affinity(struct irq_domain *domain,
return 0;
}
+ mutex_lock(&irq_domain_mutex);
+
/* Check if mapping already exists */
virq = irq_find_mapping(domain, hwirq);
if (virq) {
pr_debug("existing mapping on virq %d\n", virq);
- return virq;
+ goto out;
}
- return __irq_create_mapping_affinity(domain, hwirq, affinity);
+ virq = __irq_create_mapping_affinity(domain, hwirq, affinity);
+out:
+ mutex_unlock(&irq_domain_mutex);
+
+ return virq;
}
EXPORT_SYMBOL_GPL(irq_create_mapping_affinity);
@@ -802,6 +811,8 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
if (WARN_ON(type & ~IRQ_TYPE_SENSE_MASK))
type &= IRQ_TYPE_SENSE_MASK;
+ mutex_lock(&irq_domain_mutex);
+
/*
* If we've already configured this interrupt,
* don't do it again, or hell will break loose.
@@ -814,7 +825,7 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
* interrupt number.
*/
if (type == IRQ_TYPE_NONE || type == irq_get_trigger_type(virq))
- return virq;
+ goto out;
/*
* If the trigger type has not been set yet, then set
@@ -823,36 +834,43 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
irq_data = irq_get_irq_data(virq);
if (!irq_data)
- return 0;
+ goto err;
irqd_set_trigger_type(irq_data, type);
- return virq;
+ goto out;
}
pr_warn("type mismatch, failed to map hwirq-%lu for %s!\n",
hwirq, of_node_full_name(to_of_node(fwspec->fwnode)));
- return 0;
+ goto err;
}
if (irq_domain_is_hierarchy(domain)) {
- virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, fwspec);
+ virq = ___irq_domain_alloc_irqs(domain, -1, 1, NUMA_NO_NODE,
+ fwspec, false, NULL);
if (virq <= 0)
- return 0;
+ goto err;
} else {
/* Create mapping */
virq = __irq_create_mapping_affinity(domain, hwirq, NULL);
if (!virq)
- return virq;
+ goto err;
}
irq_data = irq_get_irq_data(virq);
if (WARN_ON(!irq_data))
- return 0;
+ goto err;
/* Store trigger type */
irqd_set_trigger_type(irq_data, type);
+out:
+ mutex_unlock(&irq_domain_mutex);
return virq;
+err:
+ mutex_unlock(&irq_domain_mutex);
+
+ return 0;
}
EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);
@@ -1877,6 +1895,13 @@ void irq_domain_set_info(struct irq_domain *domain, unsigned int virq,
irq_set_handler_data(virq, handler_data);
}
+static int ___irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
+ unsigned int nr_irqs, int node, void *arg,
+ bool realloc, const struct irq_affinity_desc *affinity)
+{
+ return -EINVAL;
+}
+
static void irq_domain_check_hierarchy(struct irq_domain *domain)
{
}
--
2.37.4
The global irq_domain_mutex is held when mapping interrupts from
non-hierarchical domains but currently not when disposing them.
This specifically means that updates of the domain mapcount is racy
(currently only used for statistics in debugfs).
Make sure to hold the global irq_domain_mutex also when disposing
mappings from non-hierarchical domains.
Fixes: 9dc6be3d4193 ("genirq/irqdomain: Add map counter")
Signed-off-by: Johan Hovold <[email protected]>
---
kernel/irq/irqdomain.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index b2087f55a1ac..23f5919e58b7 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -537,6 +537,9 @@ static void irq_domain_disassociate(struct irq_domain *domain, unsigned int irq)
return;
hwirq = irq_data->hwirq;
+
+ mutex_lock(&irq_domain_mutex);
+
irq_set_status_flags(irq, IRQ_NOREQUEST);
/* remove chip and handler */
@@ -556,6 +559,8 @@ static void irq_domain_disassociate(struct irq_domain *domain, unsigned int irq)
/* Clear reverse map for this hwirq */
irq_domain_clear_mapping(domain, hwirq);
+
+ mutex_unlock(&irq_domain_mutex);
}
static int __irq_domain_associate(struct irq_domain *domain, unsigned int virq,
--
2.37.4
Drop some unnecessary brackets that were left in place when the
corresponding code was updated.
Signed-off-by: Johan Hovold <[email protected]>
---
kernel/irq/irqdomain.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index fe9ec53fe7aa..dfd60bd49109 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -219,9 +219,8 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
domain->host_data = host_data;
domain->hwirq_max = hwirq_max;
- if (direct_max) {
+ if (direct_max)
domain->flags |= IRQ_DOMAIN_FLAG_NO_MAP;
- }
domain->revmap_size = size;
@@ -615,9 +614,8 @@ void irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
pr_debug("%s(%s, irqbase=%i, hwbase=%i, count=%i)\n", __func__,
of_node_full_name(of_node), irq_base, (int)hwirq_base, count);
- for (i = 0; i < count; i++) {
+ for (i = 0; i < count; i++)
irq_domain_associate(domain, irq_base + i, hwirq_base + i);
- }
}
EXPORT_SYMBOL_GPL(irq_domain_associate_many);
--
2.37.4
The sanity check for an already mapped virq was done outside of the
irq_domain_mutex-protected section which meant that an (unlikely) racing
association may not be detected.
Fix this by factoring out the association implementation, which will
also be used in follow-on changes to rework the locking.
Fixes: ddaf144c61da ("irqdomain: Refactor irq_domain_associate_many()")
Signed-off-by: Johan Hovold <[email protected]>
---
kernel/irq/irqdomain.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index dfd60bd49109..b2087f55a1ac 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -558,8 +558,8 @@ static void irq_domain_disassociate(struct irq_domain *domain, unsigned int irq)
irq_domain_clear_mapping(domain, hwirq);
}
-int irq_domain_associate(struct irq_domain *domain, unsigned int virq,
- irq_hw_number_t hwirq)
+static int __irq_domain_associate(struct irq_domain *domain, unsigned int virq,
+ irq_hw_number_t hwirq)
{
struct irq_data *irq_data = irq_get_irq_data(virq);
int ret;
@@ -572,7 +572,6 @@ int irq_domain_associate(struct irq_domain *domain, unsigned int virq,
if (WARN(irq_data->domain, "error: virq%i is already associated", virq))
return -EINVAL;
- mutex_lock(&irq_domain_mutex);
irq_data->hwirq = hwirq;
irq_data->domain = domain;
if (domain->ops->map) {
@@ -589,19 +588,29 @@ int irq_domain_associate(struct irq_domain *domain, unsigned int virq,
}
irq_data->domain = NULL;
irq_data->hwirq = 0;
- mutex_unlock(&irq_domain_mutex);
return ret;
}
}
domain->mapcount++;
irq_domain_set_mapping(domain, hwirq, irq_data);
- mutex_unlock(&irq_domain_mutex);
irq_clear_status_flags(virq, IRQ_NOREQUEST);
return 0;
}
+
+int irq_domain_associate(struct irq_domain *domain, unsigned int virq,
+ irq_hw_number_t hwirq)
+{
+ int ret;
+
+ mutex_lock(&irq_domain_mutex);
+ ret = __irq_domain_associate(domain, virq, hwirq);
+ mutex_unlock(&irq_domain_mutex);
+
+ return ret;
+}
EXPORT_SYMBOL_GPL(irq_domain_associate);
void irq_domain_associate_many(struct irq_domain *domain, unsigned int irq_base,
--
2.37.4
Use the irq_domain_create_hierarchy() helper to create the hierarchical
domain, which both serves as documentation and avoids poking at
irqdomain internals.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/irqchip/irq-gic-v2m.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v2m.c b/drivers/irqchip/irq-gic-v2m.c
index f4d7eeb13951..f1e75b35a52a 100644
--- a/drivers/irqchip/irq-gic-v2m.c
+++ b/drivers/irqchip/irq-gic-v2m.c
@@ -287,15 +287,14 @@ static __init int gicv2m_allocate_domains(struct irq_domain *parent)
if (!v2m)
return 0;
- inner_domain = irq_domain_create_tree(v2m->fwnode,
- &gicv2m_domain_ops, v2m);
+ inner_domain = irq_domain_create_hierarchy(parent, 0, 0, v2m->fwnode,
+ &gicv2m_domain_ops, v2m);
if (!inner_domain) {
pr_err("Failed to create GICv2m domain\n");
return -ENOMEM;
}
irq_domain_update_bus_token(inner_domain, DOMAIN_BUS_NEXUS);
- inner_domain->parent = parent;
pci_domain = pci_msi_create_irq_domain(v2m->fwnode,
&gicv2m_msi_domain_info,
inner_domain);
--
2.37.4
The IRQ domain structures are currently protected by the global
irq_domain_mutex. Switch to using more fine-grained per-domain locking,
which may potentially speed up parallel probing somewhat.
Note that the domain lock of the root domain (innermost domain) must be
used for hierarchical domains. For non-hierarchical domain (as for root
domains), the new root pointer is set to the domain itself so that
domain->root->mutex can be used in shared code paths.
Also note that hierarchical domains should be constructed using
irq_domain_create_hierarchy() (or irq_domain_add_hierarchy()) to avoid
poking at irqdomain internals.
Signed-off-by: Johan Hovold <[email protected]>
---
include/linux/irqdomain.h | 4 ++++
kernel/irq/irqdomain.c | 48 ++++++++++++++++++++++-----------------
2 files changed, 31 insertions(+), 21 deletions(-)
diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 16399de00b48..cad47737a052 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -125,6 +125,8 @@ struct irq_domain_chip_generic;
* core code.
* @flags: Per irq_domain flags
* @mapcount: The number of mapped interrupts
+ * @mutex: Domain lock, hierarhical domains use root domain's lock
+ * @root: Pointer to root domain, or containing structure if non-hierarchical
*
* Optional elements:
* @fwnode: Pointer to firmware node associated with the irq_domain. Pretty easy
@@ -152,6 +154,8 @@ struct irq_domain {
void *host_data;
unsigned int flags;
unsigned int mapcount;
+ struct mutex mutex;
+ struct irq_domain *root;
/* Optional data */
struct fwnode_handle *fwnode;
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 6f2b8a1248e1..3faea8b66120 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -217,6 +217,7 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
/* Fill structure */
INIT_RADIX_TREE(&domain->revmap_tree, GFP_KERNEL);
+ mutex_init(&domain->mutex);
domain->ops = ops;
domain->host_data = host_data;
domain->hwirq_max = hwirq_max;
@@ -227,6 +228,7 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
domain->revmap_size = size;
irq_domain_check_hierarchy(domain);
+ domain->root = domain;
mutex_lock(&irq_domain_mutex);
debugfs_add_domain_dir(domain);
@@ -503,7 +505,7 @@ static bool irq_domain_is_nomap(struct irq_domain *domain)
static void irq_domain_clear_mapping(struct irq_domain *domain,
irq_hw_number_t hwirq)
{
- lockdep_assert_held(&irq_domain_mutex);
+ lockdep_assert_held(&domain->root->mutex);
if (irq_domain_is_nomap(domain))
return;
@@ -518,7 +520,7 @@ static void irq_domain_set_mapping(struct irq_domain *domain,
irq_hw_number_t hwirq,
struct irq_data *irq_data)
{
- lockdep_assert_held(&irq_domain_mutex);
+ lockdep_assert_held(&domain->root->mutex);
if (irq_domain_is_nomap(domain))
return;
@@ -540,7 +542,7 @@ static void irq_domain_disassociate(struct irq_domain *domain, unsigned int irq)
hwirq = irq_data->hwirq;
- mutex_lock(&irq_domain_mutex);
+ mutex_lock(&domain->mutex);
irq_set_status_flags(irq, IRQ_NOREQUEST);
@@ -562,7 +564,7 @@ static void irq_domain_disassociate(struct irq_domain *domain, unsigned int irq)
/* Clear reverse map for this hwirq */
irq_domain_clear_mapping(domain, hwirq);
- mutex_unlock(&irq_domain_mutex);
+ mutex_unlock(&domain->mutex);
}
static int __irq_domain_associate(struct irq_domain *domain, unsigned int virq,
@@ -612,9 +614,9 @@ int irq_domain_associate(struct irq_domain *domain, unsigned int virq,
{
int ret;
- mutex_lock(&irq_domain_mutex);
+ mutex_lock(&domain->mutex);
ret = __irq_domain_associate(domain, virq, hwirq);
- mutex_unlock(&irq_domain_mutex);
+ mutex_unlock(&domain->mutex);
return ret;
}
@@ -731,7 +733,7 @@ unsigned int irq_create_mapping_affinity(struct irq_domain *domain,
return 0;
}
- mutex_lock(&irq_domain_mutex);
+ mutex_lock(&domain->mutex);
/* Check if mapping already exists */
virq = irq_find_mapping(domain, hwirq);
@@ -742,7 +744,7 @@ unsigned int irq_create_mapping_affinity(struct irq_domain *domain,
virq = __irq_create_mapping_affinity(domain, hwirq, affinity);
out:
- mutex_unlock(&irq_domain_mutex);
+ mutex_unlock(&domain->mutex);
return virq;
}
@@ -811,7 +813,7 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
if (WARN_ON(type & ~IRQ_TYPE_SENSE_MASK))
type &= IRQ_TYPE_SENSE_MASK;
- mutex_lock(&irq_domain_mutex);
+ mutex_lock(&domain->root->mutex);
/*
* If we've already configured this interrupt,
@@ -864,11 +866,11 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
/* Store trigger type */
irqd_set_trigger_type(irq_data, type);
out:
- mutex_unlock(&irq_domain_mutex);
+ mutex_unlock(&domain->root->mutex);
return virq;
err:
- mutex_unlock(&irq_domain_mutex);
+ mutex_unlock(&domain->root->mutex);
return 0;
}
@@ -1132,6 +1134,7 @@ struct irq_domain *irq_domain_create_hierarchy(struct irq_domain *parent,
else
domain = irq_domain_create_tree(fwnode, ops, host_data);
if (domain) {
+ domain->root = parent->root;
domain->parent = parent;
domain->flags |= flags;
}
@@ -1528,10 +1531,10 @@ int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
return -EINVAL;
}
- mutex_lock(&irq_domain_mutex);
+ mutex_lock(&domain->root->mutex);
ret = ___irq_domain_alloc_irqs(domain, irq_base, nr_irqs, node, arg,
realloc, affinity);
- mutex_unlock(&irq_domain_mutex);
+ mutex_unlock(&domain->root->mutex);
return ret;
}
@@ -1542,7 +1545,7 @@ static void irq_domain_fix_revmap(struct irq_data *d)
{
void __rcu **slot;
- lockdep_assert_held(&irq_domain_mutex);
+ lockdep_assert_held(&d->domain->root->mutex);
if (irq_domain_is_nomap(d->domain))
return;
@@ -1608,7 +1611,7 @@ int irq_domain_push_irq(struct irq_domain *domain, int virq, void *arg)
if (!parent_irq_data)
return -ENOMEM;
- mutex_lock(&irq_domain_mutex);
+ mutex_lock(&domain->root->mutex);
/* Copy the original irq_data. */
*parent_irq_data = *irq_data;
@@ -1636,7 +1639,7 @@ int irq_domain_push_irq(struct irq_domain *domain, int virq, void *arg)
irq_domain_fix_revmap(parent_irq_data);
irq_domain_set_mapping(domain, irq_data->hwirq, irq_data);
error:
- mutex_unlock(&irq_domain_mutex);
+ mutex_unlock(&domain->root->mutex);
return rv;
}
@@ -1691,7 +1694,7 @@ int irq_domain_pop_irq(struct irq_domain *domain, int virq)
if (WARN_ON(!parent_irq_data))
return -EINVAL;
- mutex_lock(&irq_domain_mutex);
+ mutex_lock(&domain->root->mutex);
irq_data->parent_data = NULL;
@@ -1703,7 +1706,7 @@ int irq_domain_pop_irq(struct irq_domain *domain, int virq)
irq_domain_fix_revmap(irq_data);
- mutex_unlock(&irq_domain_mutex);
+ mutex_unlock(&domain->root->mutex);
kfree(parent_irq_data);
@@ -1719,17 +1722,20 @@ EXPORT_SYMBOL_GPL(irq_domain_pop_irq);
void irq_domain_free_irqs(unsigned int virq, unsigned int nr_irqs)
{
struct irq_data *data = irq_get_irq_data(virq);
+ struct irq_domain *domain;
int i;
if (WARN(!data || !data->domain || !data->domain->ops->free,
"NULL pointer, cannot free irq\n"))
return;
- mutex_lock(&irq_domain_mutex);
+ domain = data->domain;
+
+ mutex_lock(&domain->root->mutex);
for (i = 0; i < nr_irqs; i++)
irq_domain_remove_irq(virq + i);
- irq_domain_free_irqs_hierarchy(data->domain, virq, nr_irqs);
- mutex_unlock(&irq_domain_mutex);
+ irq_domain_free_irqs_hierarchy(domain, virq, nr_irqs);
+ mutex_unlock(&domain->root->mutex);
irq_domain_free_irq_data(virq, nr_irqs);
irq_free_descs(virq, nr_irqs);
--
2.37.4
Use the irq_domain_create_hierarchy() helper to create the hierarchical
domain, which both serves as documentation and avoids poking at
irqdomain internals.
Signed-off-by: Johan Hovold <[email protected]>
---
arch/x86/platform/uv/uv_irq.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c
index 1a536a187d74..ee21d6a36a80 100644
--- a/arch/x86/platform/uv/uv_irq.c
+++ b/arch/x86/platform/uv/uv_irq.c
@@ -166,10 +166,9 @@ static struct irq_domain *uv_get_irq_domain(void)
if (!fn)
goto out;
- uv_domain = irq_domain_create_tree(fn, &uv_domain_ops, NULL);
- if (uv_domain)
- uv_domain->parent = x86_vector_domain;
- else
+ uv_domain = irq_domain_create_hierarchy(x86_vector_domain, 0, 0, fn,
+ &uv_domain_ops, NULL);
+ if (!uv_domain)
irq_domain_free_fwnode(fn);
out:
mutex_unlock(&uv_lock);
--
2.37.4
Use the irq_domain_create_hierarchy() helper to create the hierarchical
domain, which both serves as documentation and avoids poking at
irqdomain internals.
Signed-off-by: Johan Hovold <[email protected]>
---
arch/x86/kernel/apic/io_apic.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index a868b76cd3d4..9cc4c8e0c3c4 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2364,9 +2364,9 @@ static int mp_irqdomain_create(int ioapic)
return -ENODEV;
}
- ip->irqdomain = irq_domain_create_linear(fn, hwirqs, cfg->ops,
- (void *)(long)ioapic);
-
+ ip->irqdomain = irq_domain_create_hierarchy(parent, 0, hwirqs, fn,
+ cfg->ops,
+ (void *)(long)ioapic);
if (!ip->irqdomain) {
/* Release fw handle if it was allocated above */
if (!cfg->dev)
@@ -2374,8 +2374,6 @@ static int mp_irqdomain_create(int ioapic)
return -ENOMEM;
}
- ip->irqdomain->parent = parent;
-
if (cfg->type == IOAPIC_DOMAIN_LEGACY ||
cfg->type == IOAPIC_DOMAIN_STRICT)
ioapic_dynirq_base = max(ioapic_dynirq_base,
--
2.37.4
Use the irq_domain_add_hierarchy() helper to create the hierarchical
domain, which both serves as documentation and avoids poking at
irqdomain internals.
Signed-off-by: Johan Hovold <[email protected]>
---
drivers/irqchip/irq-alpine-msi.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/irqchip/irq-alpine-msi.c b/drivers/irqchip/irq-alpine-msi.c
index 5ddb8e578ac6..604459372fdd 100644
--- a/drivers/irqchip/irq-alpine-msi.c
+++ b/drivers/irqchip/irq-alpine-msi.c
@@ -204,16 +204,14 @@ static int alpine_msix_init_domains(struct alpine_msix_data *priv,
return -ENXIO;
}
- middle_domain = irq_domain_add_tree(NULL,
- &alpine_msix_middle_domain_ops,
- priv);
+ middle_domain = irq_domain_add_hierarchy(gic_domain, 0, 0, NULL,
+ &alpine_msix_middle_domain_ops,
+ priv);
if (!middle_domain) {
pr_err("Failed to create the MSIX middle domain\n");
return -ENOMEM;
}
- middle_domain->parent = gic_domain;
-
msi_domain = pci_msi_create_irq_domain(of_node_to_fwnode(node),
&alpine_msix_domain_info,
middle_domain);
--
2.37.4
Refactor __irq_domain_alloc_irqs() so that it can can be called
internally while holding the irq_domain_mutex.
This will be used to fix a shared-interrupt mapping race.
Signed-off-by: Johan Hovold <[email protected]>
---
kernel/irq/irqdomain.c | 88 +++++++++++++++++++++++-------------------
1 file changed, 48 insertions(+), 40 deletions(-)
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 894bc6ee6348..d6139b0218d4 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1430,40 +1430,12 @@ int irq_domain_alloc_irqs_hierarchy(struct irq_domain *domain,
return domain->ops->alloc(domain, irq_base, nr_irqs, arg);
}
-/**
- * __irq_domain_alloc_irqs - Allocate IRQs from domain
- * @domain: domain to allocate from
- * @irq_base: allocate specified IRQ number if irq_base >= 0
- * @nr_irqs: number of IRQs to allocate
- * @node: NUMA node id for memory allocation
- * @arg: domain specific argument
- * @realloc: IRQ descriptors have already been allocated if true
- * @affinity: Optional irq affinity mask for multiqueue devices
- *
- * Allocate IRQ numbers and initialized all data structures to support
- * hierarchy IRQ domains.
- * Parameter @realloc is mainly to support legacy IRQs.
- * Returns error code or allocated IRQ number
- *
- * The whole process to setup an IRQ has been split into two steps.
- * The first step, __irq_domain_alloc_irqs(), is to allocate IRQ
- * descriptor and required hardware resources. The second step,
- * irq_domain_activate_irq(), is to program the hardware with preallocated
- * resources. In this way, it's easier to rollback when failing to
- * allocate resources.
- */
-int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
- unsigned int nr_irqs, int node, void *arg,
- bool realloc, const struct irq_affinity_desc *affinity)
+static int ___irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
+ unsigned int nr_irqs, int node, void *arg,
+ bool realloc, const struct irq_affinity_desc *affinity)
{
int i, ret, virq;
- if (domain == NULL) {
- domain = irq_default_domain;
- if (WARN(!domain, "domain is NULL; cannot allocate IRQ\n"))
- return -EINVAL;
- }
-
if (realloc && irq_base >= 0) {
virq = irq_base;
} else {
@@ -1482,24 +1454,18 @@ int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
goto out_free_desc;
}
- mutex_lock(&irq_domain_mutex);
ret = irq_domain_alloc_irqs_hierarchy(domain, virq, nr_irqs, arg);
- if (ret < 0) {
- mutex_unlock(&irq_domain_mutex);
+ if (ret < 0)
goto out_free_irq_data;
- }
for (i = 0; i < nr_irqs; i++) {
ret = irq_domain_trim_hierarchy(virq + i);
- if (ret) {
- mutex_unlock(&irq_domain_mutex);
+ if (ret)
goto out_free_irq_data;
- }
}
-
+
for (i = 0; i < nr_irqs; i++)
irq_domain_insert_irq(virq + i);
- mutex_unlock(&irq_domain_mutex);
return virq;
@@ -1509,6 +1475,48 @@ int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
irq_free_descs(virq, nr_irqs);
return ret;
}
+
+/**
+ * __irq_domain_alloc_irqs - Allocate IRQs from domain
+ * @domain: domain to allocate from
+ * @irq_base: allocate specified IRQ number if irq_base >= 0
+ * @nr_irqs: number of IRQs to allocate
+ * @node: NUMA node id for memory allocation
+ * @arg: domain specific argument
+ * @realloc: IRQ descriptors have already been allocated if true
+ * @affinity: Optional irq affinity mask for multiqueue devices
+ *
+ * Allocate IRQ numbers and initialized all data structures to support
+ * hierarchy IRQ domains.
+ * Parameter @realloc is mainly to support legacy IRQs.
+ * Returns error code or allocated IRQ number
+ *
+ * The whole process to setup an IRQ has been split into two steps.
+ * The first step, __irq_domain_alloc_irqs(), is to allocate IRQ
+ * descriptor and required hardware resources. The second step,
+ * irq_domain_activate_irq(), is to program the hardware with preallocated
+ * resources. In this way, it's easier to rollback when failing to
+ * allocate resources.
+ */
+int __irq_domain_alloc_irqs(struct irq_domain *domain, int irq_base,
+ unsigned int nr_irqs, int node, void *arg,
+ bool realloc, const struct irq_affinity_desc *affinity)
+{
+ int ret;
+
+ if (domain == NULL) {
+ domain = irq_default_domain;
+ if (WARN(!domain, "domain is NULL; cannot allocate IRQ\n"))
+ return -EINVAL;
+ }
+
+ mutex_lock(&irq_domain_mutex);
+ ret = ___irq_domain_alloc_irqs(domain, irq_base, nr_irqs, node, arg,
+ realloc, affinity);
+ mutex_unlock(&irq_domain_mutex);
+
+ return ret;
+}
EXPORT_SYMBOL_GPL(__irq_domain_alloc_irqs);
/* The irq_data was moved, fix the revmap to refer to the new location */
--
2.37.4
Johan!
On Fri, Dec 09 2022 at 15:01, Johan Hovold wrote:
> Parallel probing (e.g. due to asynchronous probing) of devices that
> share interrupts can currently result in two mappings for the same
> hardware interrupt to be created.
>
> This series fixes this mapping race and clean up the irqdomain locking
> so that in the end the global irq_domain_mutex is only used for managing
> the likewise global irq_domain_list, while domain operations (e.g.
> IRQ allocations) use per-domain (hierarchy) locking.
Can you please rebase that on top of:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core
Thanks,
tglx
Hi Thomas,
On Fri, Dec 09, 2022 at 04:51:00PM +0100, Thomas Gleixner wrote:
> Johan!
>
> On Fri, Dec 09 2022 at 15:01, Johan Hovold wrote:
> > Parallel probing (e.g. due to asynchronous probing) of devices that
> > share interrupts can currently result in two mappings for the same
> > hardware interrupt to be created.
> >
> > This series fixes this mapping race and clean up the irqdomain locking
> > so that in the end the global irq_domain_mutex is only used for managing
> > the likewise global irq_domain_list, while domain operations (e.g.
> > IRQ allocations) use per-domain (hierarchy) locking.
>
> Can you please rebase that on top of:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core
The series is based on next-20221208 which should contain that branch in
its current state if I'm not mistaken.
I just tried applying it on top of irq/core and did not notice any
problems.
Johan
On Fri, Dec 09 2022 at 17:16, Johan Hovold wrote:
> On Fri, Dec 09, 2022 at 04:51:00PM +0100, Thomas Gleixner wrote:
>> On Fri, Dec 09 2022 at 15:01, Johan Hovold wrote:
>> > Parallel probing (e.g. due to asynchronous probing) of devices that
>> > share interrupts can currently result in two mappings for the same
>> > hardware interrupt to be created.
>> >
>> > This series fixes this mapping race and clean up the irqdomain locking
>> > so that in the end the global irq_domain_mutex is only used for managing
>> > the likewise global irq_domain_list, while domain operations (e.g.
>> > IRQ allocations) use per-domain (hierarchy) locking.
>>
>> Can you please rebase that on top of:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core
>
> The series is based on next-20221208 which should contain that branch in
> its current state if I'm not mistaken.
>
> I just tried applying it on top of irq/core and did not notice any
> problems.
Sorry for the noise. Pilot error. -ETOOMANYBRANCHES :)
On Fri, Dec 09 2022 at 15:01, Johan Hovold wrote:
> The IRQ domain structures are currently protected by the global
> irq_domain_mutex. Switch to using more fine-grained per-domain locking,
> which may potentially speed up parallel probing somewhat.
>
> Note that the domain lock of the root domain (innermost domain) must be
> used for hierarchical domains. For non-hierarchical domain (as for root
> domains), the new root pointer is set to the domain itself so that
> domain->root->mutex can be used in shared code paths.
>
> Also note that hierarchical domains should be constructed using
> irq_domain_create_hierarchy() (or irq_domain_add_hierarchy()) to avoid
> poking at irqdomain internals.
While I agree in principle, this change really makes me nervous.
Any fail in setting up domain->root correctly, e.g. by not using
irq_domain_create_hierarchy(), cannot be detected and creates nasty to
debug race conditions.
So we need some debug mechanism which allows to validate that
domain->root is set up correctly when domain->parent != NULL.
Thanks,
tglx
On Mon, Dec 12, 2022 at 03:14:34PM +0100, Thomas Gleixner wrote:
> On Fri, Dec 09 2022 at 15:01, Johan Hovold wrote:
> > The IRQ domain structures are currently protected by the global
> > irq_domain_mutex. Switch to using more fine-grained per-domain locking,
> > which may potentially speed up parallel probing somewhat.
> >
> > Note that the domain lock of the root domain (innermost domain) must be
> > used for hierarchical domains. For non-hierarchical domain (as for root
> > domains), the new root pointer is set to the domain itself so that
> > domain->root->mutex can be used in shared code paths.
> >
> > Also note that hierarchical domains should be constructed using
> > irq_domain_create_hierarchy() (or irq_domain_add_hierarchy()) to avoid
> > poking at irqdomain internals.
>
> While I agree in principle, this change really makes me nervous.
>
> Any fail in setting up domain->root correctly, e.g. by not using
> irq_domain_create_hierarchy(), cannot be detected and creates nasty to
> debug race conditions.
>
> So we need some debug mechanism which allows to validate that
> domain->root is set up correctly when domain->parent != NULL.
Lockdep will catch that due to the
lockdep_assert_held(&domain->root->mutex);
I added to irq_domain_set_mapping() and which is is called for each
(inner) domain in a hierarchy when allocating an IRQ.
Johan
On Mon, Dec 12 2022 at 15:29, Johan Hovold wrote:
> On Mon, Dec 12, 2022 at 03:14:34PM +0100, Thomas Gleixner wrote:
>> On Fri, Dec 09 2022 at 15:01, Johan Hovold wrote:
>> > The IRQ domain structures are currently protected by the global
>> > irq_domain_mutex. Switch to using more fine-grained per-domain locking,
>> > which may potentially speed up parallel probing somewhat.
>> >
>> > Note that the domain lock of the root domain (innermost domain) must be
>> > used for hierarchical domains. For non-hierarchical domain (as for root
>> > domains), the new root pointer is set to the domain itself so that
>> > domain->root->mutex can be used in shared code paths.
>> >
>> > Also note that hierarchical domains should be constructed using
>> > irq_domain_create_hierarchy() (or irq_domain_add_hierarchy()) to avoid
>> > poking at irqdomain internals.
>>
>> While I agree in principle, this change really makes me nervous.
>>
>> Any fail in setting up domain->root correctly, e.g. by not using
>> irq_domain_create_hierarchy(), cannot be detected and creates nasty to
>> debug race conditions.
>>
>> So we need some debug mechanism which allows to validate that
>> domain->root is set up correctly when domain->parent != NULL.
>
> Lockdep will catch that due to the
>
> lockdep_assert_held(&domain->root->mutex);
>
> I added to irq_domain_set_mapping() and which is is called for each
> (inner) domain in a hierarchy when allocating an IRQ.
Hmm. Indeed that should do the trick.
Some comments would be appreciated as the rules around domain->root are
far from obvious.
Thanks,
tglx
On 9/12/22 15:01, Johan Hovold wrote:
> Use the irq_domain_create_hierarchy() helper to create the hierarchical
> domain, which both serves as documentation and avoids poking at
> irqdomain internals.
>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> drivers/irqchip/irq-gic-v2m.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
On 9/12/22 15:01, Johan Hovold wrote:
> Drop some unnecessary brackets that were left in place when the
> corresponding code was updated.
>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> kernel/irq/irqdomain.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
On 9/12/22 15:01, Johan Hovold wrote:
> Use the irq_domain_create_hierarchy() helper to create the hierarchical
> domain, which both serves as documentation and avoids poking at
> irqdomain internals.
>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> arch/x86/kernel/apic/io_apic.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
On 9/12/22 15:01, Johan Hovold wrote:
> Use the irq_domain_create_hierarchy() helper to create the hierarchical
> domain, which both serves as documentation and avoids poking at
> irqdomain internals.
>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> arch/x86/platform/uv/uv_irq.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
On 9/12/22 15:01, Johan Hovold wrote:
> Use the irq_domain_add_hierarchy() helper to create the hierarchical
> domain, which both serves as documentation and avoids poking at
> irqdomain internals.
>
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> drivers/irqchip/irq-alpine-msi.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
On Thu, Dec 15, 2022 at 5:22 PM Johan Hovold <[email protected]> wrote:
>
> Parallel probing (e.g. due to asynchronous probing) of devices that
> share interrupts can currently result in two mappings for the same
> hardware interrupt to be created.
>
> This series fixes this mapping race and clean up the irqdomain locking
> so that in the end the global irq_domain_mutex is only used for managing
> the likewise global irq_domain_list, while domain operations (e.g.
> IRQ allocations) use per-domain (hierarchy) locking.
>
> Johan
>
>
> Changes in v2
> - split out redundant-lookup cleanup (1/4)
> - use a per-domain mutex to address mapping race (2/4)
> - move kernel-doc to exported function (2/4)
> - fix association race (3/4, new)
> - use per-domain mutex for associations (4/4, new)
>
> Changes in v3
> - drop dead and bogus code (1--3/19, new)
> - fix racy mapcount accesses (5/19, new)
> - drop revmap mutex (6/19, new)
> - use irq_domain_mutex to address mapping race (9/19)
> - clean up irq_domain_push/pop_irq() (10/19, new)
> - use irq_domain_create_hierarchy() to construct hierarchies
> (11--18/19, new)
> - switch to per-domain locking (19/19, new)
>
>
> Johan Hovold (19):
> irqdomain: Drop bogus fwspec-mapping error handling
> irqdomain: Drop dead domain-name assignment
> irqdomain: Drop leftover brackets
> irqdomain: Fix association race
> irqdomain: Fix disassociation race
> irqdomain: Drop revmap mutex
> irqdomain: Look for existing mapping only once
> irqdomain: Refactor __irq_domain_alloc_irqs()
> irqdomain: Fix mapping-creation race
> irqdomain: Clean up irq_domain_push/pop_irq()
> x86/ioapic: Use irq_domain_create_hierarchy()
> x86/apic: Use irq_domain_create_hierarchy()
> irqchip/alpine-msi: Use irq_domain_add_hierarchy()
> irqchip/gic-v2m: Use irq_domain_create_hierarchy()
> irqchip/gic-v3-its: Use irq_domain_create_hierarchy()
> irqchip/gic-v3-mbi: Use irq_domain_create_hierarchy()
> irqchip/loongson-pch-msi: Use irq_domain_create_hierarchy()
> irqchip/mvebu-odmi: Use irq_domain_create_hierarchy()
> irqdomain: Switch to per-domain locking
>
> arch/x86/kernel/apic/io_apic.c | 8 +-
> arch/x86/platform/uv/uv_irq.c | 7 +-
> drivers/irqchip/irq-alpine-msi.c | 8 +-
> drivers/irqchip/irq-gic-v2m.c | 5 +-
> drivers/irqchip/irq-gic-v3-its.c | 13 +-
> drivers/irqchip/irq-gic-v3-mbi.c | 5 +-
> drivers/irqchip/irq-loongson-pch-msi.c | 9 +-
> drivers/irqchip/irq-mvebu-odmi.c | 13 +-
> include/linux/irqdomain.h | 6 +-
> kernel/irq/irqdomain.c | 328 ++++++++++++++-----------
> 10 files changed, 220 insertions(+), 182 deletions(-)
>
> --
Tested-by: Hsin-Yi Wang <[email protected]>
The series solves a race issue when having non-populated 2nd source
components that share the same irq on ARM devices:
Previously we would see
[ 0.476357] irq: type mismatch, failed to map hwirq-11 for pinctrl@10005000!
and the component failed to probe.
> 2.37.4
>
> Parallel probing (e.g. due to asynchronous probing) of devices that
> share interrupts can currently result in two mappings for the same
> hardware interrupt to be created.
>
> This series fixes this mapping race and clean up the irqdomain locking
> so that in the end the global irq_domain_mutex is only used for managing
> the likewise global irq_domain_list, while domain operations (e.g.
> IRQ allocations) use per-domain (hierarchy) locking.
>
> Johan
>
>
> Changes in v2
> - split out redundant-lookup cleanup (1/4)
> - use a per-domain mutex to address mapping race (2/4)
> - move kernel-doc to exported function (2/4)
> - fix association race (3/4, new)
> - use per-domain mutex for associations (4/4, new)
>
> Changes in v3
> - drop dead and bogus code (1--3/19, new)
> - fix racy mapcount accesses (5/19, new)
> - drop revmap mutex (6/19, new)
> - use irq_domain_mutex to address mapping race (9/19)
> - clean up irq_domain_push/pop_irq() (10/19, new)
> - use irq_domain_create_hierarchy() to construct hierarchies
> (11--18/19, new)
> - switch to per-domain locking (19/19, new)
>
>
> Johan Hovold (19):
> irqdomain: Drop bogus fwspec-mapping error handling
> irqdomain: Drop dead domain-name assignment
> irqdomain: Drop leftover brackets
> irqdomain: Fix association race
> irqdomain: Fix disassociation race
> irqdomain: Drop revmap mutex
> irqdomain: Look for existing mapping only once
> irqdomain: Refactor __irq_domain_alloc_irqs()
> irqdomain: Fix mapping-creation race
> irqdomain: Clean up irq_domain_push/pop_irq()
> x86/ioapic: Use irq_domain_create_hierarchy()
> x86/apic: Use irq_domain_create_hierarchy()
> irqchip/alpine-msi: Use irq_domain_add_hierarchy()
> irqchip/gic-v2m: Use irq_domain_create_hierarchy()
> irqchip/gic-v3-its: Use irq_domain_create_hierarchy()
> irqchip/gic-v3-mbi: Use irq_domain_create_hierarchy()
> irqchip/loongson-pch-msi: Use irq_domain_create_hierarchy()
> irqchip/mvebu-odmi: Use irq_domain_create_hierarchy()
> irqdomain: Switch to per-domain locking
>
> arch/x86/kernel/apic/io_apic.c | 8 +-
> arch/x86/platform/uv/uv_irq.c | 7 +-
> drivers/irqchip/irq-alpine-msi.c | 8 +-
> drivers/irqchip/irq-gic-v2m.c | 5 +-
> drivers/irqchip/irq-gic-v3-its.c | 13 +-
> drivers/irqchip/irq-gic-v3-mbi.c | 5 +-
> drivers/irqchip/irq-loongson-pch-msi.c | 9 +-
> drivers/irqchip/irq-mvebu-odmi.c | 13 +-
> include/linux/irqdomain.h | 6 +-
> kernel/irq/irqdomain.c | 328 ++++++++++++++-----------
> 10 files changed, 220 insertions(+), 182 deletions(-)
>
> --
> 2.37.4
Tested-by: Mark-PK Tsai <[email protected]>
We have the same issue and this patch series fix that.
Thanks!
Link: https://lore.kernel.org/lkml/[email protected]/
On Mon, Dec 12 2022 at 17:18, Thomas Gleixner wrote:
> On Mon, Dec 12 2022 at 15:29, Johan Hovold wrote:
>> I added to irq_domain_set_mapping() and which is is called for each
>> (inner) domain in a hierarchy when allocating an IRQ.
>
> Hmm. Indeed that should do the trick.
>
> Some comments would be appreciated as the rules around domain->root are
> far from obvious.
Any update on this one?
On Wed, Jan 11, 2023 at 07:28:35PM +0100, Thomas Gleixner wrote:
> On Mon, Dec 12 2022 at 17:18, Thomas Gleixner wrote:
> > On Mon, Dec 12 2022 at 15:29, Johan Hovold wrote:
> >> I added to irq_domain_set_mapping() and which is is called for each
> >> (inner) domain in a hierarchy when allocating an IRQ.
> >
> > Hmm. Indeed that should do the trick.
> >
> > Some comments would be appreciated as the rules around domain->root are
> > far from obvious.
>
> Any update on this one?
Sorry about the delay. I'll take a look at this tomorrow.
Johan
On Thu, Dec 15, 2022 at 05:31:24PM +0800, Hsin-Yi Wang wrote:
> On Thu, Dec 15, 2022 at 5:22 PM Johan Hovold <[email protected]> wrote:
> >
> > Parallel probing (e.g. due to asynchronous probing) of devices that
> > share interrupts can currently result in two mappings for the same
> > hardware interrupt to be created.
> >
> > This series fixes this mapping race and clean up the irqdomain locking
> > so that in the end the global irq_domain_mutex is only used for managing
> > the likewise global irq_domain_list, while domain operations (e.g.
> > IRQ allocations) use per-domain (hierarchy) locking.
> Tested-by: Hsin-Yi Wang <[email protected]>
>
> The series solves a race issue when having non-populated 2nd source
> components that share the same irq on ARM devices:
> Previously we would see
> [ 0.476357] irq: type mismatch, failed to map hwirq-11 for pinctrl@10005000!
> and the component failed to probe.
Thanks again for testing. I just sent a v4 adding a couple of clarifying
comments to the final patch.
Johan
On Tue, Dec 20, 2022 at 11:30:42AM +0800, Mark-PK Tsai wrote:
> > Parallel probing (e.g. due to asynchronous probing) of devices that
> > share interrupts can currently result in two mappings for the same
> > hardware interrupt to be created.
> >
> > This series fixes this mapping race and clean up the irqdomain locking
> > so that in the end the global irq_domain_mutex is only used for managing
> > the likewise global irq_domain_list, while domain operations (e.g.
> > IRQ allocations) use per-domain (hierarchy) locking.
> Tested-by: Mark-PK Tsai <[email protected]>
>
> We have the same issue and this patch series fix that.
> Thanks!
>
> Link: https://lore.kernel.org/lkml/[email protected]/
Thanks for confirming. I just sent a v4 with a couple of clarifying
comments added to the final patch.
Johan