next/mmotm unbootable on G5: irqdomain

next/mmotm unbootable on G5: irqdomain

[Hugh Dickins]

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).

Hugh

commit 94f036a1f242f98cc30700b7676c07270a9c5c27
Author: Grant Likely <grant.likely@secretlab.ca>
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@secretlab.ca>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Milton Miller <miltonm@bga.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Cc: Rob Herring <rob.herring@calxeda.com>

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);
 

Re: next/mmotm unbootable on G5: irqdomain

[Benjamin Herrenschmidt]

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@secretlab.ca>
> 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@secretlab.ca>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Milton Miller <miltonm@bga.com>
> Cc: Paul Mundt <lethal@linux-sh.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> 
> 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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: next/mmotm unbootable on G5: irqdomain

[Hugh Dickins]

On Sun, 22 Jul 2012, Benjamin Herrenschmidt wrote:
> 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 !).
> 
> > commit 94f036a1f242f98cc30700b7676c07270a9c5c27
> > Author: Grant Likely <grant.likely@secretlab.ca>
> > Date:   Sun Jun 3 22:04:39 2012 -0700
> > 
> > irqdomain: eliminate slow-path revmap lookups

Thanks, Ben - here's my /proc/cpuinfo:

processor	: 0
cpu		: PPC970MP, altivec supported
clock		: 2500.000000MHz
revision	: 1.1 (pvr 0044 0101)

processor	: 1
cpu		: PPC970MP, altivec supported
clock		: 2500.000000MHz
revision	: 1.1 (pvr 0044 0101)

processor	: 2
cpu		: PPC970MP, altivec supported
clock		: 2500.000000MHz
revision	: 1.1 (pvr 0044 0101)

processor	: 3
cpu		: PPC970MP, altivec supported
clock		: 2500.000000MHz
revision	: 1.1 (pvr 0044 0101)

timebase	: 33333333
platform	: PowerMac
model		: PowerMac11,2
machine		: PowerMac11,2
motherboard	: PowerMac11,2 MacRISC4 Power Macintosh 
detected as	: 337 (PowerMac G5 Dual Core)
pmac flags	: 00000000
L2 cache	: 1024K unified
pmac-generation	: NewWorld

Hugh

Re: next/mmotm unbootable on G5: irqdomain

[Benjamin Herrenschmidt]

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).

This fixes it (Grant, how do we avoid bisection breakage here ? I can
put that in -powerpc and we can make sure that gets merged before your
tree ?)

powerpc/mpic: Create a revmap with enough entries for IPIs and timers

The current mpic code creates a linear revmap just big enough for all
the sources, which happens to miss the IPIs and timers on some machines.

This will in turn break when the irqdomain code loses the fallback of
doing a linear search when the revmap fails (and really slows down IPIs
otherwise).

This happens for example on the U4 based Apple machines such as the
dual core PowerMac G5s.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 906f29c..bfc6211 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1376,7 +1376,7 @@ struct mpic * __init mpic_alloc(struct device_node *node,
 	mpic->isu_mask = (1 << mpic->isu_shift) - 1;
 
 	mpic->irqhost = irq_domain_add_linear(mpic->node,
-				       last_irq + 1,
+				       intvec_top,
 				       &mpic_host_ops, mpic);
 
 	/*

More irqdomain problems (Was: next/mmotm unbootable on G5: irqdomain)

[Benjamin Herrenschmidt]

Allright, another one Grant:

unsigned int irq_find_mapping(struct irq_domain *domain,
			      irq_hw_number_t hwirq)
{
	struct irq_data *data;

	/* Look for default domain if nececssary */
	if (domain == NULL)
		domain = irq_default_domain;
	if (domain == NULL)
		return 0;

	switch (domain->revmap_type) {
	case IRQ_DOMAIN_MAP_LEGACY:
		return irq_domain_legacy_revmap(domain, hwirq);
	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;
+               return 0;
	case IRQ_DOMAIN_MAP_NOMAP:

Please, stick a proper commit message and my s-o-b and see if you can fix
your tree before you ask Linus to pull because that's not pretty on any
pseries .... irq_find_mapping() does get called for all interrupt the
first time it's mapped to check if there's a pre-existing mapping, so
the case of the thing being unpopulated is absolutely legit.

the NOMAP case has a similar dubious exit case but since I'm not that
familiar with NOMAP I haven't touched it.

Cheers,
Ben.

Re: next/mmotm unbootable on G5: irqdomain

[Hugh Dickins]

On Mon, 23 Jul 2012, Benjamin Herrenschmidt wrote:
> 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).
> 
> This fixes it

Confirmed: many thanks, Ben.

> (Grant, how do we avoid bisection breakage here ? I can
> put that in -powerpc and we can make sure that gets merged before your
> tree ?)
> 
> powerpc/mpic: Create a revmap with enough entries for IPIs and timers
> 
> The current mpic code creates a linear revmap just big enough for all
> the sources, which happens to miss the IPIs and timers on some machines.
> 
> This will in turn break when the irqdomain code loses the fallback of
> doing a linear search when the revmap fails (and really slows down IPIs
> otherwise).
> 
> This happens for example on the U4 based Apple machines such as the
> dual core PowerMac G5s.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 906f29c..bfc6211 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -1376,7 +1376,7 @@ struct mpic * __init mpic_alloc(struct device_node *node,
>  	mpic->isu_mask = (1 << mpic->isu_shift) - 1;
>  
>  	mpic->irqhost = irq_domain_add_linear(mpic->node,
> -				       last_irq + 1,
> +				       intvec_top,
>  				       &mpic_host_ops, mpic);
>  
>  	/*
> 
> 
> 

Re: next/mmotm unbootable on G5: irqdomain

[Grant Likely]

On Sun, Jul 22, 2012 at 8:45 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> 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).
>
> This fixes it (Grant, how do we avoid bisection breakage here ? I can
> put that in -powerpc and we can make sure that gets merged before your
> tree ?)

My tree must be rebased to eliminate bisect breakage. The existing
commits in my tree have the breakage, and fiddling with the merge
order doesn't affect that. I don't want to rebase though. The safest
approach (smallest window of breakage) is to apply that fix onto my
irqdomain tree.

g.

>
> powerpc/mpic: Create a revmap with enough entries for IPIs and timers
>
> The current mpic code creates a linear revmap just big enough for all
> the sources, which happens to miss the IPIs and timers on some machines.
>
> This will in turn break when the irqdomain code loses the fallback of
> doing a linear search when the revmap fails (and really slows down IPIs
> otherwise).
>
> This happens for example on the U4 based Apple machines such as the
> dual core PowerMac G5s.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 906f29c..bfc6211 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -1376,7 +1376,7 @@ struct mpic * __init mpic_alloc(struct device_node *node,
>         mpic->isu_mask = (1 << mpic->isu_shift) - 1;
>
>         mpic->irqhost = irq_domain_add_linear(mpic->node,
> -                                      last_irq + 1,
> +                                      intvec_top,
>                                        &mpic_host_ops, mpic);
>
>         /*
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: next/mmotm unbootable on G5: irqdomain

[Benjamin Herrenschmidt]

On Mon, 2012-07-23 at 01:59 -0600, Grant Likely wrote:
> My tree must be rebased to eliminate bisect breakage. The existing
> commits in my tree have the breakage, and fiddling with the merge
> order doesn't affect that. I don't want to rebase though. The safest
> approach (smallest window of breakage) is to apply that fix onto my
> irqdomain tree.

With your other breakage on pseries I'm thinking rebasing might be the
only option...

Cheers,
Ben.

Re: next/mmotm unbootable on G5: irqdomain

[Grant Likely]

On Mon, Jul 23, 2012 at 4:26 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Mon, 2012-07-23 at 01:59 -0600, Grant Likely wrote:
>> My tree must be rebased to eliminate bisect breakage. The existing
>> commits in my tree have the breakage, and fiddling with the merge
>> order doesn't affect that. I don't want to rebase though. The safest
>> approach (smallest window of breakage) is to apply that fix onto my
>> irqdomain tree.
>
> With your other breakage on pseries I'm thinking rebasing might be the
> only option...

Fair enough. I'm not planning to ask Linus to pull for a few days yet
anyway. I've been pretty useless as a kernel maintainer for the last 3
months so I want to give a bit more time in linux-next to catch
fallout before it gets merged.

As-is I'm backing off from the linear/legacy/tree merge patch as just
too risky. I've already pulled that stuff out of linux-next.

g.

Re: next/mmotm unbootable on G5: irqdomain

[Grant Likely]

On Mon, Jul 23, 2012 at 4:31 PM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Mon, Jul 23, 2012 at 4:26 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>> On Mon, 2012-07-23 at 01:59 -0600, Grant Likely wrote:
>>> My tree must be rebased to eliminate bisect breakage. The existing
>>> commits in my tree have the breakage, and fiddling with the merge
>>> order doesn't affect that. I don't want to rebase though. The safest
>>> approach (smallest window of breakage) is to apply that fix onto my
>>> irqdomain tree.
>>
>> With your other breakage on pseries I'm thinking rebasing might be the
>> only option...
>
> Fair enough. I'm not planning to ask Linus to pull for a few days yet
> anyway. I've been pretty useless as a kernel maintainer for the last 3
> months so I want to give a bit more time in linux-next to catch
> fallout before it gets merged.
>
> As-is I'm backing off from the linear/legacy/tree merge patch as just
> too risky. I've already pulled that stuff out of linux-next.

Can I pull you pseries fix into my tree (my preference), or do I need
to rebase on top of yours?

g.

Re: next/mmotm unbootable on G5: irqdomain

[Benjamin Herrenschmidt]

On Mon, 2012-07-23 at 16:32 -0600, Grant Likely wrote:
> > As-is I'm backing off from the linear/legacy/tree merge patch as just
> > too risky. I've already pulled that stuff out of linux-next.
> 
> Can I pull you pseries fix into my tree (my preference), or do I need
> to rebase on top of yours? 

The mpic fix for the g5 is in Linus tree already, I added it on top of
powerpc -next before I asked Linus to pull.

For pseries (ie the fix for irq_find_mapping vs. radix), I don't have a
formal patch, just the one I hand typed in my previous email, so do
whatever you want with it.

Cheers,
Ben.

Re: next/mmotm unbootable on G5: irqdomain

[Grant Likely]

On Mon, Jul 23, 2012 at 9:21 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Mon, 2012-07-23 at 16:32 -0600, Grant Likely wrote:
>> > As-is I'm backing off from the linear/legacy/tree merge patch as just
>> > too risky. I've already pulled that stuff out of linux-next.
>>
>> Can I pull you pseries fix into my tree (my preference), or do I need
>> to rebase on top of yours?
>
> The mpic fix for the g5 is in Linus tree already, I added it on top of
> powerpc -next before I asked Linus to pull.
>
> For pseries (ie the fix for irq_find_mapping vs. radix), I don't have a
> formal patch, just the one I hand typed in my previous email, so do
> whatever you want with it.

Okay, I'll merge in Linus' tree at the appropriate point to protect
against bisection, and I'll fix up the appropriate patch that touches
irq_find_mapping.

g.

Re: More irqdomain problems (Was: next/mmotm unbootable on G5: irqdomain)

[Grant Likely]

On Mon, Jul 23, 2012 at 12:32 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Allright, another one Grant:
>
> unsigned int irq_find_mapping(struct irq_domain *domain,
>                               irq_hw_number_t hwirq)
> {
>         struct irq_data *data;
>
>         /* Look for default domain if nececssary */
>         if (domain == NULL)
>                 domain = irq_default_domain;
>         if (domain == NULL)
>                 return 0;
>
>         switch (domain->revmap_type) {
>         case IRQ_DOMAIN_MAP_LEGACY:
>                 return irq_domain_legacy_revmap(domain, hwirq);
>         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;
> +               return 0;
>         case IRQ_DOMAIN_MAP_NOMAP:
>
> Please, stick a proper commit message and my s-o-b and see if you can fix
> your tree before you ask Linus to pull because that's not pretty on any
> pseries .... irq_find_mapping() does get called for all interrupt the
> first time it's mapped to check if there's a pre-existing mapping, so
> the case of the thing being unpopulated is absolutely legit.
>
> the NOMAP case has a similar dubious exit case but since I'm not that
> familiar with NOMAP I haven't touched it.

I've decided to rework the patch to simply omit the WARN statement. It
isn't really needed. I've merged in Linus' tree below the eliminate
slow-path patch and remove the WARN statement. It's been pushed out to
my irqdomain/next branch, so it should show up in tomorrow's
linux-next.

You can find it here if you want to give it a spin:

git://git.secretlab.ca/git/linux-2.6 irqdomain/next

I'll give it a bit more time in linux-next before I ask Linus to pull.

g.