Xen devel - Mar 2006 - [patch] bitops on irq_cpustat_t->__softirq_pending

As mentioned earlier, PowerPC''s atomic ops operate on longs, and we
have made
our *_bit() prototypes use long* (instead of void*) to warn us of problems at 
compile time. Here''s one caller that was flagged:
	test_and_set_bit(nr, &softirq_pending(cpu))
Accordingly, we need __softirq_pending to be long, not int.

PowerPC is currently using a few files unmodified from the x86 versions; 
hardirq.h is one of them. We could copy asm-x86/hardirq.h and change the type 
of this one field, but that leads to maintenance headaches down the road.

Instead, this patch changes asm-x86/hardirq.h to use a long where PPC needs 
it. This doesn''t change the size of the structure for x86. There are
other
ways we could and should do this code-sharing, but this one is the least 
impact (and this is an area where IA64''s divergence is potentially 
problematic).

I think it would be a good idea to make a wiki page that covers the files that 
are candidates for sharing. I know Jimi has investigated this subject...

Please apply.

-- 
Hollis Blanchard
IBM Linux Technology Center

Use long instead of int for irq_cpustat_t->__softirq_pending, allowing
PowerPC
to share this file unchanged (PowerPC bitops operate on longs).

Signed-off-by: Jimi Xenidis <jimix@watson.ibm.com>
Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>

diff -r 50778f42f2dd xen/include/asm-x86/hardirq.h
--- a/xen/include/asm-x86/hardirq.h	Wed Mar 29 16:02:40 2006 +0100
+++ b/xen/include/asm-x86/hardirq.h	Wed Mar 29 16:23:32 2006 -0600
@@ -5,7 +5,7 @@
 #include <xen/cache.h>
 
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned long __softirq_pending;
 	unsigned int __local_irq_count;
 	unsigned int __nmi_count;
 	unsigned long idle_timestamp;

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

>From: Hollis Blanchard
>Sent: 2006年3月31日 0:45
>Instead, this patch changes asm-x86/hardirq.h to use a long where PPC
>needs
>it. This doesn''t change the size of the structure for x86. There
are other
>ways we could and should do this code-sharing, but this one is the least
>impact (and this is an area where IA64''s divergence is potentially
>problematic).
Xen/IA64 has percpu hardware pages support and so softirq pending 
indicator is put together with other more percpu stuffs placed on the 
percpu pages. That can accelerate percpu access a lot. 

Another concern is, IA64 supports atomic bitops with different width, just 
like x86 while long is double size of integer on IA64. If the similar 
approach in this patch is used in other common places, it will add 
unnecessary size to xen/ia64. 

Thanks,
Kevin
>
>I think it would be a good idea to make a wiki page that covers the files
>that
>are candidates for sharing. I know Jimi has investigated this subject...
>
Agree.

Thanks,
Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Mar 30, 2006, at 7:25 PM, Tian, Kevin wrote:
>> From: Hollis Blanchard
>> Sent: 2006年3月31日 0:45
>> Instead, this patch changes asm-x86/hardirq.h to use a long where PPC
>> needs
>> it. This doesn''t change the size of the structure for x86.
There are
>> other
>> ways we could and should do this code-sharing, but this one is the 
>> least
>> impact (and this is an area where IA64''s divergence is
potentially
>> problematic).
>
> Xen/IA64 has percpu hardware pages support and so softirq pending
> indicator is put together with other more percpu stuffs placed on the
> percpu pages. That can accelerate percpu access a lot.
Makes sense. In fact I would say any array with NR_CPUS elements should 
probably be in a per-cpu data area to avoid "__cacheline_aligned" 
padding.

However, we don''t have a standard infrastructure for that in Xen right 
now, and that means that IA64''s hardirq.h, part of the weird 
suck-in-headers-from-Linux thing, is radically different from x86 (and 
thus PPC). And *that* means it''s quite difficult to make any changes to
hardirq.h without fear of breaking the IA64 build.
> Another concern is, IA64 supports atomic bitops with different width, 
> just
> like x86 while long is double size of integer on IA64. If the similar
> approach in this patch is used in other common places, it will add
> unnecessary size to xen/ia64.
Understood.
>> I think it would be a good idea to make a wiki page that covers the 
>> files
>> that are candidates for sharing. I know Jimi has investigated this 
>> subject...
>
> Agree.
Jimi, want to write down your thoughts and reply with the URL?

-- 
Hollis Blanchard
IBM Linux Technology Center


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 31 Mar 2006, at 13:03, Hollis Blanchard wrote:
> Makes sense. In fact I would say any array with NR_CPUS elements 
> should probably be in a per-cpu data area to avoid 
> "__cacheline_aligned" padding.
>
> However, we don''t have a standard infrastructure for that in Xen
right
> now, and that means that IA64''s hardirq.h, part of the weird 
> suck-in-headers-from-Linux thing, is radically different from x86 (and 
> thus PPC). And *that* means it''s quite difficult to make any
changes
> to hardirq.h without fear of breaking the IA64 build.
I''m going to pull in percpu support after 3.0.2.

  -- Keir


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel