Re: [PATCH] Fix interrupt distribution in ppc970

[Milton Miller <miltonm@bga.com>]

On May 3, 2007, at 9:47 AM, Mohan Kumar M wrote:
> On Thu, Apr 26, 2007 at 09:42:50AM -0500, Milton Miller wrote:
>> Yes.   The whole point of
>>> -static int get_irq_server(unsigned int virq)
>>> +static int get_irq_server(unsigned int virq, unsigned int
>>> strict_check)
>> was to factor out the common code in this function.
>
> Milton,
>
> How about this patch?

Getting closer, still not right.

>
> Index: linux-2.6.21.1/arch/powerpc/platforms/pseries/xics.c
> ===================================================================
> --- linux-2.6.21.1.orig/arch/powerpc/platforms/pseries/xics.c
> +++ linux-2.6.21.1/arch/powerpc/platforms/pseries/xics.c
> @@ -156,9 +156,9 @@ static inline void lpar_qirr_info(int n_
>
>
>  #ifdef CONFIG_SMP
> -static int get_irq_server(unsigned int virq)
> +static int get_irq_server(unsigned int virq, unsigned int 
> strict_check)
>  {
> -	unsigned int server;
> +	int server;
>  	/* For the moment only implement delivery to all cpus or one cpu */
>  	cpumask_t cpumask = irq_desc[virq].affinity;
>  	cpumask_t tmp = CPU_MASK_NONE;
> @@ -166,22 +166,28 @@ static int get_irq_server(unsigned int v
>  	if (!distribute_irqs)
>  		return default_server;
>
> -	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
> -		server = default_distrib_server;
> -	} else {
> +	if (!cpus_equal(cpumask, CPU_MASK_ALL)) {
>  		cpus_and(tmp, cpu_online_map, cpumask);
>
> -		if (cpus_empty(tmp))
> -			server = default_distrib_server;
> +		server = first_cpu(tmp);
> +
> +		if (server < NR_CPUS)
> +			return get_hard_smp_processor_id(server);
> +		else {
> +			if (strict_check)
> +				return -1;


> +			else
> +				return default_distrib_server;
> +		}
> +	} else {

Take out the above 4 lines, so that the return always has cpu_online
vs cpu_present factored in, by falling through from specific mask to
default server selection.

> +		if (cpus_equal(cpu_online_map, cpu_present_map))
> +			return default_distrib_server;
>  		else
> -			server = get_hard_smp_processor_id(first_cpu(tmp));
> +			return default_server;
>  	}

[This matching brace will go and indent will change.]

> -
> -	return server;
> -
>  }
...

> @@ -398,8 +404,7 @@ static void xics_set_affinity(unsigned i
>  	unsigned int irq;
>  	int status;
>  	int xics_status[2];
> -	unsigned long newmask;
> -	cpumask_t tmp = CPU_MASK_NONE;
> +	int irq_server;
>
>  	irq = (unsigned int)irq_map[virq].hwirq;
>  	if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
> @@ -413,18 +418,28 @@ static void xics_set_affinity(unsigned i
>  		return;
>  	}
>
> -	/* For the moment only implement delivery to all cpus or one cpu */
> -	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
> -		newmask = default_distrib_server;
> -	} else {
> -		cpus_and(tmp, cpu_online_map, cpumask);
> -		if (cpus_empty(tmp))
> +	/* Get current irq_server for the given irq */
> +	irq_server = get_irq_server(irq, 1);
> +	if (irq_server == -1) {
> +		printk(KERN_ERR "xics_set_affinity: Invalid cpumask\n");

WARNING or NOTICE would be fine.  The interrupt number should be 
printed.
and maybe "No online cpus in <cpumask> for irq %d" would be better (I
think there is a print cpumask helper).

> +		return;
> +	}
> +
> +	/* For the moment only implement delivery to all cpus or one cpu.
> +	 * Compare the irq_server with the new cpumask. If the irq_server
> +	 * is specified in cpumask, do the required rtas_call, otherwise
> +	 * return by printing an error message
> +	 */
> +	if (!cpus_equal(cpumask, CPU_MASK_ALL)) {
> +		if (!cpu_isset(irq_server, cpumask)) {
> +			printk(KERN_ERR "xics_set_affinity: Invalid "
> +							"cpumask\n");
>  			return;

I don't understand what you are trying to do here.  We already
chose the mask, and printed an error if we got -1 due to strict.
I think this can just be dropped?

> -		newmask = get_hard_smp_processor_id(first_cpu(tmp));
> +		}
>  	}
>
>  	status = rtas_call(ibm_set_xive, 3, 1, NULL,
> -				irq, newmask, xics_status[1]);
> +				irq, irq_server, xics_status[1]);
>
>  	if (status) {
>  		printk(KERN_ERR "xics_set_affinity: irq=%u ibm,set-xive "


milton


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec