[parisc-linux] ATOMIC_HASH brainfart

[parisc-linux] ATOMIC_HASH brainfart

[Grant Grundler]

Hi all,
Seems like someone was confused when they hacked ATOMIC_HASH macro.
Cacheline is 64 bytes not 256...but I've been told several times gcc
is smart enough to convert "foo/64" to "foo>>6"...then just use
the L1_CACHE_BYTES definition to indicate intent?

I haven't verified everything builds and works right
but it looks "obviously correct" to me. Comments?

I'm still looking at getting SMP to boot.

grant


Index: include/asm/atomic.h
===================================================================
RCS file: /var/cvs/linux-2.6/include/asm-parisc/atomic.h,v
retrieving revision 1.6
diff -u -p -r1.6 atomic.h
--- a/include/asm/atomic.h	25 Mar 2004 06:19:07 -0000	1.6
+++ b/include/asm/atomic.h	4 Apr 2004 06:48:20 -0000
@@ -19,10 +19,10 @@ typedef spinlock_t atomic_lock_t;
 
 /* Use an array of spinlocks for our atomic_ts.
  * Hash function to index into a different SPINLOCK.
- * Since "a" is usually an address, ">>8" makes one spinlock per 64-bytes.
+ * Since "a" is usually an address, use one spinlock per cacheline.
  */
 #  define ATOMIC_HASH_SIZE 4
-#  define ATOMIC_HASH(a) (&__atomic_hash[(((unsigned long) a)>>8)&(ATOMIC_HASH_SIZE-1)])
+#  define ATOMIC_HASH(a) (&__atomic_hash[(((unsigned long) a)/L1_CACHE_BYTES)&(ATOMIC_HASH_SIZE-1)])
 
 extern atomic_lock_t __atomic_hash[ATOMIC_HASH_SIZE] __lock_aligned;
 

Re: [parisc-linux] ATOMIC_HASH brainfart

[Alan Cox]

On Sul, 2004-04-04 at 07:56, Grant Grundler wrote:
> Hi all,
> Seems like someone was confused when they hacked ATOMIC_HASH macro.
> Cacheline is 64 bytes not 256...but I've been told several times gcc
> is smart enough to convert "foo/64" to "foo>>6"...then just use
> the L1_CACHE_BYTES definition to indicate intent?

IFF foo is a constant or foo is unsigned. Looks fine to me

Re: [parisc-linux] ATOMIC_HASH brainfart

[Randolph Chung]

> > Seems like someone was confused when they hacked ATOMIC_HASH macro.
> > Cacheline is 64 bytes not 256...but I've been told several times gcc
> > is smart enough to convert "foo/64" to "foo>>6"...then just use
> > the L1_CACHE_BYTES definition to indicate intent?
> 
> IFF foo is a constant or foo is unsigned. Looks fine to me

or you can even use L1_CACHE_SHIFT :)

randolph
-- 
Randolph Chung
Debian GNU/Linux Developer, hppa/ia64 ports
http://www.tausq.org/