Re: [RFC PATCH] sparse_irq aka dyn_irq

[Cyrill Gorcunov <gorcunov@gmail.com>]

[Yinghai Lu - Sat, Nov 08, 2008 at 11:05:55PM -0800]
| 
| impact: new feature sparseirq
| 
| for sparse_irq, irq_desc, and irq_cfg is not using list_head to chain up
| also not add per_cpu_dyn_array... no user now
| 
| add some kind of hash table as Ingo suggesting.
| remove dyna_array, and enable sparse_irq by default, use kzalloc_node to get it
| use desc->chip_data for x86 to store irq_cfg
| make irq_desc to go with affinity aka irq_desc moving etc
| only call move_irq_desc in irq_complete_move() --- but it seems not trigger that moving.
| 
| Signed-off-by: Yinghai Lu <yinghai@kernel.org>
| 
| ---

Hi Yinghai,

from a glance view (didn't read the whole patch)

...
|  
| -static struct irq_cfg *irq_cfg_alloc(unsigned int irq)
| +static struct irq_cfg *get_one_free_irq_cfg(int cpu)
|  {
| -	return irq_cfg(irq);
| +	struct irq_cfg *cfg;
| +	int node;
| +
| +	if (cpu < 0)
| +		cpu = smp_processor_id();
| +	node = cpu_to_node(cpu);
| +
| +	cfg = kzalloc_node(sizeof(*cfg), GFP_KERNEL, node);
| +	printk(KERN_DEBUG "  alloc irq_cfg on cpu %d node %d\n", cpu, node);
| +
| +	return cfg;
|  }
|  
| -/*
| - * Rough estimation of how many shared IRQs there are, can be changed
| - * anytime.
| - */
| -#define MAX_PLUS_SHARED_IRQS NR_IRQS
| -#define PIN_MAP_SIZE (MAX_PLUS_SHARED_IRQS + NR_IRQS)
| +static void free_irq_cfg(struct irq_cfg *cfg)
| +{
| +	kfree(cfg);
| +}
| +
| +void arch_init_chip_data(struct irq_desc *desc, int cpu)
| +{
| +	struct irq_cfg *cfg;
| +
| +	cfg = desc->chip_data;
| +	if (!cfg)
| +		desc->chip_data = get_one_free_irq_cfg(cpu);
| +}
| +
| +static void init_copy_irq_2_pin(struct irq_cfg *old_cfg, struct irq_cfg *cfg,
| +				 int cpu);
| +
| +void arch_init_copy_chip_data(struct irq_desc *old_desc,
| +				 struct irq_desc *desc, int cpu)
| +{
| +	struct irq_cfg *cfg;
| +	struct irq_cfg *old_cfg;
| +
| +	cfg = get_one_free_irq_cfg(cpu);
| +	desc->chip_data = cfg;
| +
| +	old_cfg = old_desc->chip_data;
| +
| +	memcpy(cfg, old_cfg, sizeof(struct irq_cfg));

If cfg gets NULL here we will be NULL-dereferring
(cause of possible kzalloc_node fails).

| +
| +	init_copy_irq_2_pin(old_cfg, cfg, cpu);
| +}
| +
...

Am I missgin something?

		- Cyrill -