lkml.org 
[lkml]   [2008]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2008-11-28 17:39    [W:0.068 / U:0.584 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site