Re: next/mmotm unbootable on G5: irqdomain

From: Benjamin Herrenschmidt
Date: Sun Jul 22 2012 - 09:09:53 EST


On Sat, 2012-07-21 at 19:47 -0700, Hugh Dickins wrote:
> I have to revert the patch below from mmotm 2012-07-20-16-30 or
> next-20120720 in order to boot on the PowerPC G5: otherwise it
> freezes before switching to the framebuffer console - but I'm
> not certain where because that initial console doesn't scroll
> (there are mpic messages at bottom and at top of screen, probably
> later messages at the top but I don't know the sequence).

Remind me your G5 variant ? (/proc/cpuinfo will do). I'll have a look
tomorrow (and thanks for testing !).

Cheers,
Ben.

> Hugh
>
> commit 94f036a1f242f98cc30700b7676c07270a9c5c27
> Author: Grant Likely <grant.likely@xxxxxxxxxxxx>
> Date: Sun Jun 3 22:04:39 2012 -0700
>
> irqdomain: eliminate slow-path revmap lookups
>
> With the current state of irq_domain, the reverse map is always updated
> when new IRQs get mapped. This means that the irq_find_mapping() function
> can be simplified to execute the revmap lookup functions unconditionally
>
> This patch adds lookup functions for the revmaps that don't yet have one
> and removes the slow path lookup code path.
>
> v8: Broke out unrelated changes into separate patches. Rebased on Paul's irq
> association patches.
> v7: Rebased to irqdomain/next for v3.4 and applied before the removal of 'hint'
> v6: Remove the slow path entirely. The only place where the slow path
> could get called is for a linear mapping if the hwirq number is larger
> than the linear revmap size. There shouldn't be any interrupt
> controllers that do that.
> v5: rewrite to not use a ->revmap() callback. It is simpler, smaller,
> safer and faster to open code each of the revmap lookups directly into
> irq_find_mapping() via a switch statement.
> v4: Fix build failure on incorrect variable reference.
>
> Signed-off-by: Grant Likely <grant.likely@xxxxxxxxxxxx>
> Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Milton Miller <miltonm@xxxxxxx>
> Cc: Paul Mundt <lethal@xxxxxxxxxxxx>
> Cc: Rob Herring <rob.herring@xxxxxxxxxxx>
>
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index c0e638b..a9b810e 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -686,16 +686,11 @@ EXPORT_SYMBOL_GPL(irq_dispose_mapping);
> * irq_find_mapping() - Find a linux irq from an hw irq number.
> * @domain: domain owning this hardware interrupt
> * @hwirq: hardware irq number in that domain space
> - *
> - * This is a slow path, for use by generic code. It's expected that an
> - * irq controller implementation directly calls the appropriate low level
> - * mapping function.
> */
> unsigned int irq_find_mapping(struct irq_domain *domain,
> irq_hw_number_t hwirq)
> {
> - unsigned int i;
> - unsigned int hint = hwirq % nr_irqs;
> + struct irq_data *data;
>
> /* Look for default domain if nececssary */
> if (domain == NULL)
> @@ -703,22 +698,27 @@ unsigned int irq_find_mapping(struct irq_domain *domain,
> if (domain == NULL)
> return 0;
>
> - /* legacy -> bail early */
> - if (domain->revmap_type == IRQ_DOMAIN_MAP_LEGACY)
> + switch (domain->revmap_type) {
> + case IRQ_DOMAIN_MAP_LEGACY:
> return irq_domain_legacy_revmap(domain, hwirq);
> -
> - /* Slow path does a linear search of the map */
> - if (hint == 0)
> - hint = 1;
> - i = hint;
> - do {
> - struct irq_data *data = irq_get_irq_data(i);
> + case IRQ_DOMAIN_MAP_LINEAR:
> + return irq_linear_revmap(domain, hwirq);
> + case IRQ_DOMAIN_MAP_TREE:
> + rcu_read_lock();
> + data = radix_tree_lookup(&domain->revmap_data.tree, hwirq);
> + rcu_read_unlock();
> + if (data)
> + return data->irq;
> + break;
> + case IRQ_DOMAIN_MAP_NOMAP:
> + data = irq_get_irq_data(hwirq);
> if (data && (data->domain == domain) && (data->hwirq == hwirq))
> - return i;
> - i++;
> - if (i >= nr_irqs)
> - i = 1;
> - } while(i != hint);
> + return hwirq;
> + break;
> + }
> +
> + WARN(1, "ERROR: irq revmap went horribly wrong. revmap_type=%i\n",
> + domain->revmap_type);
> return 0;
> }
> EXPORT_SYMBOL_GPL(irq_find_mapping);
> @@ -728,32 +728,19 @@ EXPORT_SYMBOL_GPL(irq_find_mapping);
> * @domain: domain owning this hardware interrupt
> * @hwirq: hardware irq number in that domain space
> *
> - * This is a fast path, for use by irq controller code that uses linear
> - * revmaps. It does fallback to the slow path if the revmap doesn't exist
> - * yet and will create the revmap entry with appropriate locking
> + * This is a fast path that can be called directly by irq controller code to
> + * save a handful of instructions.
> */
> unsigned int irq_linear_revmap(struct irq_domain *domain,
> irq_hw_number_t hwirq)
> {
> - unsigned int *revmap;
> + BUG_ON(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR);
>
> - if (WARN_ON_ONCE(domain->revmap_type != IRQ_DOMAIN_MAP_LINEAR))
> - return irq_find_mapping(domain, hwirq);
> -
> - /* Check revmap bounds */
> - if (unlikely(hwirq >= domain->revmap_data.linear.size))
> - return irq_find_mapping(domain, hwirq);
> -
> - /* Check if revmap was allocated */
> - revmap = domain->revmap_data.linear.revmap;
> - if (unlikely(revmap == NULL))
> - return irq_find_mapping(domain, hwirq);
> -
> - /* Fill up revmap with slow path if no mapping found */
> - if (unlikely(!revmap[hwirq]))
> - revmap[hwirq] = irq_find_mapping(domain, hwirq);
> + /* Check revmap bounds; complain if exceeded */
> + if (WARN_ON(hwirq >= domain->revmap_data.linear.size))
> + return 0;
>
> - return revmap[hwirq];
> + return domain->revmap_data.linear.revmap[hwirq];
> }
> EXPORT_SYMBOL_GPL(irq_linear_revmap);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/