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.032 / U:117.684 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site