Thread (36 messages) 36 messages, 8 authors, 2007-06-15

Re: [PATCH] Fix interrupt distribution in ppc970

From: Milton Miller <hidden>
Date: 2007-04-26 14:43:19

On Apr 26, 2007, at 4:24 AM, Mohan Kumar M wrote:
On Fri, Apr 20, 2007 at 12:45:15AM -0500, Milton Miller wrote:
[snip]
Milton, I hope this patch meets all your requirements.
Closer, much better.
quoted hunk ↗ jump to hunk
Cc: Milton Miller <redacted>,
    Michael Ellerman [off-list ref]
Signed-off-by: Mohan Kumar M <redacted>
---
 arch/powerpc/platforms/pseries/xics.c |   39 
++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 15 deletions(-)

Index: linux-2.6.21-rc4/arch/powerpc/platforms/pseries/xics.c
===================================================================
--- linux-2.6.21-rc4.orig/arch/powerpc/platforms/pseries/xics.c
+++ linux-2.6.21-rc4/arch/powerpc/platforms/pseries/xics.c
@@ -156,9 +156,9 @@ static inline void lpar_qirr_info(int n_
[snipping]
+	if (!cpus_equal(cpumask, CPU_MASK_ALL)) {
 		cpus_and(tmp, cpu_online_map, cpumask);

+		server = first_cpu(tmp);
+
+		if (server < NR_CPUS)
+			return get_hard_smp_processor_id(server);
+		else {
+			if(strict_check)
+				return (-1);
No parens around the return value.  That is, use return -1;
+			else
+				return default_distrib_server;
+		}
...
quoted hunk ↗ jump to hunk
@@ -415,7 +421,10 @@ static void xics_set_affinity(unsigned i

 	/* For the moment only implement delivery to all cpus or one cpu */
 	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
-		newmask = default_distrib_server;
+		if (cpus_equal(cpu_online_map, cpu_present_map))
+			newmask = default_distrib_server;
+		else
+			newmask = default_server;
 	} else {
 		cpus_and(tmp, cpu_online_map, cpumask);
 		if (cpus_empty(tmp))

===================
quoted
...
quoted
@@ -415,7 +419,10 @@ static void xics_set_affinity(unsigned i
...
quoted
	/* For the moment only implement delivery to all cpus or one cpu */
	if (cpus_equal(cpumask, CPU_MASK_ALL)) {
-		newmask = default_distrib_server;
+		if (cpus_equal(cpu_online_map, cpu_present_map))
this was supposed to be the call with strict = 1
Do you mean to use 'strict_check' argument in xics_set_affinity?
set_affinity call is declared in linux/irq.h, so if modifying
xics_set_affinity will affect other arch's set_affinity also.
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.

I wasn't trying to change the prototype of xics_set_affinity.

Looking at the code a bit, I think part of the confusion is that newmask
is horribly misnamed.   Please rename it to server or irqserver.
Obtain its value by calling get_irq_server.  If the server returned
is -1 (in strict mode), don't call rtas (just return like today).
I guess a printk could be in order since the function is void, and
only root can request the change.

milton
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help