Messages in this thread | | | Date | Fri, 28 Nov 2008 17:34:56 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 1/2] irq: sparseirq enabling v3 |
| |
* Yinghai Lu <yinghai@kernel.org> wrote:
> +void __init arch_early_irq_init(void) > { > - return irq < nr_irqs ? irq_cfgx + irq : NULL; > + struct irq_cfg *cfg; > + struct irq_desc *desc; > + int count; > + int i; > +#ifdef CONFIG_SPARSE_IRQ > + int count_desc = NR_IRQS_LEGACY; > +#else > + int count_desc = NR_IRQS; > +#endif
could this be hidden in a .h file by creating some sort of nr_boot_irqs() inline, or a NR_BOOT_IRQS define?
> Index: linux-2.6/drivers/char/random.c > =================================================================== > --- linux-2.6.orig/drivers/char/random.c > +++ linux-2.6/drivers/char/random.c > @@ -558,6 +558,8 @@ struct timer_rand_state { > unsigned dont_count_entropy:1; > }; > > +#ifndef CONFIG_SPARSE_IRQ > + > static struct timer_rand_state *irq_timer_state[NR_IRQS]; > > static struct timer_rand_state *get_timer_rand_state(unsigned int irq) > @@ -576,6 +578,33 @@ static void set_timer_rand_state(unsigne > irq_timer_state[irq] = state; > } > > +#else > + > +static struct timer_rand_state *get_timer_rand_state(unsigned int irq) > +{ > + struct irq_desc *desc; > + > + desc = irq_to_desc(irq); > + > + if (!desc) > + return NULL; > + > + return desc->timer_rand_state; > +} > + > +static void set_timer_rand_state(unsigned int irq, struct timer_rand_state *state) > +{ > + struct irq_desc *desc; > + > + desc = irq_to_desc(irq); > + > + if (!desc) > + return; > + > + desc->timer_rand_state = state; > +} > +#endif
i'd suggest to move this into a .h file.
> +++ linux-2.6/fs/proc/stat.c > @@ -27,6 +27,9 @@ static int show_stat(struct seq_file *p, > u64 sum = 0; > struct timespec boottime; > unsigned int per_irq_sum; > +#ifdef CONFIG_GENERIC_HARDIRQS > + struct irq_desc *desc; > +#endif
Why is this define needed? If it's about a build warning, you can add something like this to defines:
(void)(param)
to make unused parameters used as well.
> +#ifdef CONFIG_SPARSE_IRQ > + seq_printf(p, " %d:%u", j, per_irq_sum); > +#else > seq_printf(p, " %u", per_irq_sum); > - } > +#endif
doesnt this change the /proc ABI ? A couple of tools would break. I think the right approach is to go from 0 to NR_IRQS-1 and print zeroes for NULL descs. I.e. a natural extension of the current scheme.
> +++ linux-2.6/fs/proc/interrupts.c > @@ -8,6 +8,23 @@ > /* > * /proc/interrupts > */ > +#ifdef CONFIG_SPARSE_IRQ > +static void *int_seq_start(struct seq_file *f, loff_t *pos) > +{ > + rcu_read_lock(); > + return seq_list_start(&sparse_irqs_head, *pos);
is this rcu-safe approach still needed?
> =================================================================== > --- linux-2.6.orig/include/linux/interrupt.h > +++ linux-2.6/include/linux/interrupt.h > @@ -18,6 +18,8 @@ > #include <asm/ptrace.h> > #include <asm/system.h> > > +extern int nr_irqs;
isnt this obsolete now?
> Index: linux-2.6/arch/x86/kernel/irq.c > =================================================================== > --- linux-2.6.orig/arch/x86/kernel/irq.c > +++ linux-2.6/arch/x86/kernel/irq.c > @@ -99,25 +99,37 @@ static int show_other_interrupts(struct > int show_interrupts(struct seq_file *p, void *v) > { > unsigned long flags, any_count = 0; > - int i = *(loff_t *) v, j; > + int i, j; > struct irqaction *action; > struct irq_desc *desc; > + int head = 0; > > +#ifdef CONFIG_SPARSE_IRQ > + desc = list_entry(v, struct irq_desc, list); > + i = desc->irq; > + if (&desc->list == sparse_irqs_head.next) > + head = 1; > +#else > + i = *(loff_t *) v; > if (i > nr_irqs) > return 0; > > if (i == nr_irqs) > return show_other_interrupts(p); > + if (i == 0) > + head = 1; > + > + desc = irq_to_desc(i); > +#endif
i dont think this has to change? We have an array of pointers, and we should extend the current loops by skipping over NULL entries.
Ingo
| |