Re: panic from vector domain patch (was RE: Linus' tree broken?)

Doug Chapman
2007-07-25T13:03:21Z

On Wed, 2007-07-25 at 17:59 +0900, Kenji Kaneshige wrote:
> Doug,
>
> 2007-07-25 (水) の 01:09 -0400 に Doug Chapman さんは書きました:
> > Yasuaki,
> >
> > I added a bunch of printk's to try to better understand this.
> > ia64_mv.setup does get initialized properly but then gets overwritten.
> > It appears that it gets overwritten in __bind_irq_vector but I have not
> > narrowed down exactly where. I put a printk at the top and bottom of
> > this function to show what ia64_mv.setup is and I am finding that on the
> > 3rd call to this function it causes ia64_mv.setup to get set to -1.
> >
> > It is getting way too late for me (1AM here). I will try to dig into
> > this more tomorrow but at least I have narrowed it down to a bit of code
> > that actually is touched by the commit that causes the panic.
> >
> > - Doug
>
> I found a bug which causes wrong access to vector_table[] array.
> I guess ia64_mv.setup was destroyed by this. Could you try the
> attached patch?
>
> Thanks,
> Kenji Kaneshige
> ---
>
> Fix wrong access to vector_table[].
>
> Signed-off-by: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
>
> ---
> arch/ia64/kernel/irq_ia64.c | 21 ++++++++++-----------
> 1 files changed, 10 insertions(+), 11 deletions(-)
>
> Index: linux-2.6.23-rc1/arch/ia64/kernel/irq_ia64.c
> =================================> --- linux-2.6.23-rc1.orig/arch/ia64/kernel/irq_ia64.c
> +++ linux-2.6.23-rc1/arch/ia64/kernel/irq_ia64.c
> @@ -85,8 +85,8 @@ DEFINE_PER_CPU(int[IA64_NUM_VECTORS], ve
> [0 ... IA64_NUM_VECTORS - 1] = IA64_SPURIOUS_INT_VECTOR
> };
>
> -static cpumask_t vector_table[IA64_MAX_DEVICE_VECTORS] = {
> - [0 ... IA64_MAX_DEVICE_VECTORS - 1] = CPU_MASK_NONE
> +static cpumask_t vector_table[IA64_NUM_VECTORS] = {
> + [0 ... IA64_NUM_VECTORS - 1] = CPU_MASK_NONE
> };
>
> static int irq_status[NR_IRQS] = {
> @@ -123,17 +123,18 @@ static inline int find_unassigned_irq(vo
> static inline int find_unassigned_vector(cpumask_t domain)
> {
> cpumask_t mask;
> - int pos;
> + int pos, vector;
>
> cpus_and(mask, domain, cpu_online_map);
> if (cpus_empty(mask))
> return -EINVAL;
>
> for (pos = 0; pos < IA64_NUM_DEVICE_VECTORS; pos++) {
> - cpus_and(mask, domain, vector_table[pos]);
> + vector = IA64_FIRST_DEVICE_VECTOR + pos;
> + cpus_and(mask, domain, vector_table[vector]);
> if (!cpus_empty(mask))
> continue;
> - return IA64_FIRST_DEVICE_VECTOR + pos;
> + return vector;
> }
> return -ENOSPC;
> }
> @@ -141,7 +142,7 @@ static inline int find_unassigned_vector
> static int __bind_irq_vector(int irq, int vector, cpumask_t domain)
> {
> cpumask_t mask;
> - int cpu, pos;
> + int cpu;
> struct irq_cfg *cfg = &irq_cfg[irq];
>
> cpus_and(mask, domain, cpu_online_map);
> @@ -156,8 +157,7 @@ static int __bind_irq_vector(int irq, in
> cfg->vector = vector;
> cfg->domain = domain;
> irq_status[irq] = IRQ_USED;
> - pos = vector - IA64_FIRST_DEVICE_VECTOR;
> - cpus_or(vector_table[pos], vector_table[pos], domain);
> + cpus_or(vector_table[vector], vector_table[vector], domain);
> return 0;
> }
>
> @@ -174,7 +174,7 @@ int bind_irq_vector(int irq, int vector,
>
> static void __clear_irq_vector(int irq)
> {
> - int vector, cpu, pos;
> + int vector, cpu;
> cpumask_t mask;
> cpumask_t domain;
> struct irq_cfg *cfg = &irq_cfg[irq];
> @@ -189,8 +189,7 @@ static void __clear_irq_vector(int irq)
> cfg->vector = IRQ_VECTOR_UNASSIGNED;
> cfg->domain = CPU_MASK_NONE;
> irq_status[irq] = IRQ_UNUSED;
> - pos = vector - IA64_FIRST_DEVICE_VECTOR;
> - cpus_andnot(vector_table[pos], vector_table[pos], domain);
> + cpus_andnot(vector_table[vector], vector_table[vector], domain);
> }
>
> static void clear_irq_vector(int irq)
>
>

Kenji,

Yes, this does fix the problem. My system is booting just fine with
this patch.

good job,

- Doug