Re: [PATCH 4/5] xen: events: dynamically allocate irq info structures

[Jeremy Fitzhardinge <jeremy@goop.org>]

 On 10/26/2010 07:30 AM, Konrad Rzeszutek Wilk wrote:
> On Mon, Oct 25, 2010 at 05:23:32PM +0100, Ian Campbell wrote:
>> Removes nr_irq sized array allocation at start of day.
>>
>> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
>> ---
>>  drivers/xen/events.c |   50 +++++++++++++++++++++++++++++++++++++++++---------
>>  1 files changed, 41 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
>> index 94055ea..9b58505 100644
>> --- a/drivers/xen/events.c
>> +++ b/drivers/xen/events.c
>> @@ -56,6 +56,8 @@
>>   */
>>  static DEFINE_SPINLOCK(irq_mapping_update_lock);
>>  
>> +static LIST_HEAD(xen_irq_list_head);
>> +
>>  /* IRQ <-> VIRQ mapping. */
>>  static DEFINE_PER_CPU(int [NR_VIRQS], virq_to_irq) = {[0 ... NR_VIRQS-1] = -1};
>>  
>> @@ -85,6 +87,7 @@ enum xen_irq_type {
>>   */
>>  struct irq_info
>>  {
>> +	struct list_head list;
>>  	enum xen_irq_type type;	/* type */
>>  	unsigned short evtchn;	/* event channel */
>>  	unsigned short cpu;	/* cpu bound */
>> @@ -103,7 +106,6 @@ struct irq_info
>>  #define PIRQ_NEEDS_EOI	(1 << 0)
>>  #define PIRQ_SHAREABLE	(1 << 1)
>>  
>> -static struct irq_info *irq_info;
>>  static int *pirq_to_irq;
>>  static int nr_pirqs;
>>  
>> @@ -132,7 +134,7 @@ static struct irq_chip xen_pirq_chip;
>>  /* Get info for IRQ */
>>  static struct irq_info *info_for_irq(unsigned irq)
>>  {
>> -	return &irq_info[irq];
>> +	return get_irq_data(irq);
>>  }
>>  
>>  /* Constructors for packed IRQ information. */
>> @@ -315,7 +317,7 @@ static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
>>  	__clear_bit(chn, cpu_evtchn_mask(cpu_from_irq(irq)));
>>  	__set_bit(chn, cpu_evtchn_mask(cpu));
>>  
>> -	irq_info[irq].cpu = cpu;
>> +	info_for_irq(irq)->cpu = cpu;
>>  }
>>  
>>  static void init_evtchn_cpu_bindings(void)
>> @@ -428,6 +430,21 @@ static int find_unbound_pirq(void)
>>  	return -1;
>>  }
>>  
>> +static void xen_irq_init(unsigned irq)
>> +{
>> +	struct irq_info *info;
>> +
>> +	info = kmalloc(sizeof(*info), GFP_KERNEL);
>> +	if (info == NULL)
>> +		panic("Unable to allocate metadata for IRQ%d\n", irq);
> There is a bunch of panic around allocating IRQs. There is one earlier
> in xen_irq_alloc too, and I was wondering - would it make sense to
> perhaps print an error to both the hypervisor and the kernel and
> just return -1 as an IRQ and let the kernel continue with its normal
> failure path?
>
> I am thinking just in terms of making the system still be able to
> work even if parts of it are busted, instead of just crashing the system.
>
> Not sure which philosophy is domiant in the Linux kernel?

Normally to print something and struggle on, rather than crash over
every little thing.  There's been a general tendency to convert BUG into
WARN, for example.

    J