Hi Jan,
On Mon, Oct 09, 2006 at 08:27:54AM +0200, Jan Kiszka wrote:
> Willy Tarreau wrote:
> > I've done it too, but unfortunately, I discovered that it does not build
> > with gcc-2.95 anymore, while 3.3 is fine :
> >
> > cyclades.c: In function `cyy_interrupt':
> > /usr/src/git/linux-2.4/include/asm/bitops.h:130: inconsistent operand constraints in an `asm'
> > cyclades.c: In function `cyz_handle_rx':
> > /usr/src/git/linux-2.4/include/asm/bitops.h:130: inconsistent operand constraints in an `asm'
> >
> > 127 __asm__ __volatile__( LOCK_PREFIX
> > 128 "btsl %2,%1\n\tsbbl %0,%0"
> > 129 :"=r" (oldbit),"+m" (ADDR)
> > 130 :"Ir" (nr) : "memory");
> >
> >
> > I don't know what the right solution should be. I'm not fond of #ifdefs,
> > and I'm embarrassed to know that gcc can do all sorts of nasty things due
> > to a wrong clobbering :-/
> >
> > If you have any idea on the subject, I'm willing to try.
>
> What about
>
> #if __GNUC__ < 3
> #define ADDR_DEPS "=m"
> #else
> #define ADDR_DEPS "+m"
> #endif
>
> __asm__ __volatile__( LOCK_PREFIX
> "btsl %2,%1\n\tsbbl %0,%0"
> :"=r" (oldbit),ADDR_DEPS (ADDR)
> :"Ir" (nr) : "memory");
>
> or something similar? Would keep the number of #ifs low.
I don't like it for two reasons :
- I don't know how gcc-3.0 to 3.2 behave on such code, so may be this
would implicitly categorize them in the wrong group
- it makes the asm code very hard to read. Operand dependencies are
very sensible tricks which need to be extremely clear.
I have searched the archives for previous occurrences of this recurring
problem, and found another alternative which is said to work on all gcc
versions. So here's the patch for both x86 and x86_64. I checked that it
produces identical code as I obtain with the patch from 2.6. It also
fixes your testcase for gcc-2.95 to 4.1.1.
Could you please give it a try ?
Andi, Linus, I'd be very interested in your comments on this patch, since
you're more used to deal with this problem than me.
Thanks in advance,
Willy
>From 695f7e0b3806ff44def385856cda60d7f1c2df1c Mon Sep 17 00:00:00 2001
From: Willy Tarreau <[email protected]>
Date: Sat, 14 Oct 2006 21:39:06 +0200
Subject: [PATCH] 2.4.x: i386/x86_64 bitops clobberings
Problem reported by Jan Kiszka <[email protected]> :
"After going through debugging hell with some out-of-tree code,
I realised that this patch :
http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=92934bcbf96bc9dc931c40ca5f1a57685b7b813b
makes a difference: current 2.6 works with the following code
sequence as expected (printk is executed), 2.4 fails. I used
gcc 3.3.6-13 (Debian).
unsigned long a = 1;
int module_init(void)
{
unsigned long b = 0;
int x;
x = __test_and_set_bit(0, &b);
if (__test_and_set_bit(0, &a))
printk("x = %d\n", x);
return -1;
}
I ported the x86 part back to 2.4.33 and my problem disappeared. "
The problem with this patch is that the use of "+m" in asm statements
triggers bugs in gcc 2.95 which is supported by kernel 2.4. For
instance, cyclades.c does not build anymore :
$ gcc -O2 -pipe -c cyclades-c.c
cyclades.c: In function `cyy_interrupt':
/usr/src/git/linux-2.4/include/asm/bitops.h:134: inconsistent operand constraints in an `asm'
cyclades.c: In function `cyz_handle_rx':
/usr/src/git/linux-2.4/include/asm/bitops.h:134: inconsistent operand constraints in an `asm'
As explained by Richard Henderson here, the equivalent construct
"=m"(x) : "m"(x) works on any version of gcc :
http://marc.theaimsgroup.com/?l=3Dlinux-kernel&m=3D107475162200773&w=3D2
I successfully verified on the example code above and on cyclades.c
that the exact same code is produced on x86 with this construct with
gcc 2.95.3, 3.3.6, 3.4.4, and 4.1.1, so it looks fine.
Just like in the former patch for 2.6, this one was applied to both
i386 and x86_64.
Signed-Off-By: Willy Tarreau <[email protected]>
---
include/asm-i386/bitops.h | 22 +++++++++++-----------
include/asm-x86_64/bitops.h | 22 +++++++++++-----------
2 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/include/asm-i386/bitops.h b/include/asm-i386/bitops.h
index 9b6b55e..bb63c10 100644
--- a/include/asm-i386/bitops.h
+++ b/include/asm-i386/bitops.h
@@ -38,7 +38,7 @@ static __inline__ void set_bit(int nr, v
__asm__ __volatile__( LOCK_PREFIX
"btsl %1,%0"
:"=m" (ADDR)
- :"Ir" (nr));
+ :"Ir" (nr), "m" (ADDR));
}
/**
@@ -55,7 +55,7 @@ static __inline__ void __set_bit(int nr,
__asm__(
"btsl %1,%0"
:"=m" (ADDR)
- :"Ir" (nr));
+ :"Ir" (nr), "m" (ADDR));
}
/**
@@ -73,7 +73,7 @@ static __inline__ void clear_bit(int nr,
__asm__ __volatile__( LOCK_PREFIX
"btrl %1,%0"
:"=m" (ADDR)
- :"Ir" (nr));
+ :"Ir" (nr), "m" (ADDR));
}
#define smp_mb__before_clear_bit() barrier()
#define smp_mb__after_clear_bit() barrier()
@@ -92,7 +92,7 @@ static __inline__ void __change_bit(int
__asm__ __volatile__(
"btcl %1,%0"
:"=m" (ADDR)
- :"Ir" (nr));
+ :"Ir" (nr), "m" (ADDR));
}
/**
@@ -109,7 +109,7 @@ static __inline__ void change_bit(int nr
__asm__ __volatile__( LOCK_PREFIX
"btcl %1,%0"
:"=m" (ADDR)
- :"Ir" (nr));
+ :"Ir" (nr), "m" (ADDR));
}
/**
@@ -127,7 +127,7 @@ static __inline__ int test_and_set_bit(i
__asm__ __volatile__( LOCK_PREFIX
"btsl %2,%1\n\tsbbl %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"Ir" (nr) : "memory");
+ :"Ir" (nr), "m" (ADDR) : "memory");
return oldbit;
}
@@ -147,7 +147,7 @@ static __inline__ int __test_and_set_bit
__asm__(
"btsl %2,%1\n\tsbbl %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"Ir" (nr));
+ :"Ir" (nr), "m" (ADDR));
return oldbit;
}
@@ -166,7 +166,7 @@ static __inline__ int test_and_clear_bit
__asm__ __volatile__( LOCK_PREFIX
"btrl %2,%1\n\tsbbl %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"Ir" (nr) : "memory");
+ :"Ir" (nr), "m" (ADDR) : "memory");
return oldbit;
}
@@ -186,7 +186,7 @@ static __inline__ int __test_and_clear_b
__asm__(
"btrl %2,%1\n\tsbbl %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"Ir" (nr));
+ :"Ir" (nr), "m" (ADDR));
return oldbit;
}
@@ -198,7 +198,7 @@ static __inline__ int __test_and_change_
__asm__ __volatile__(
"btcl %2,%1\n\tsbbl %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"Ir" (nr) : "memory");
+ :"Ir" (nr), "m" (ADDR) : "memory");
return oldbit;
}
@@ -217,7 +217,7 @@ static __inline__ int test_and_change_bi
__asm__ __volatile__( LOCK_PREFIX
"btcl %2,%1\n\tsbbl %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"Ir" (nr) : "memory");
+ :"Ir" (nr), "m" (ADDR) : "memory");
return oldbit;
}
diff --git a/include/asm-x86_64/bitops.h b/include/asm-x86_64/bitops.h
index c8a4749..f6a2a4f 100644
--- a/include/asm-x86_64/bitops.h
+++ b/include/asm-x86_64/bitops.h
@@ -38,7 +38,7 @@ static __inline__ void set_bit(long nr,
__asm__ __volatile__( LOCK_PREFIX
"btsq %1,%0"
:"=m" (ADDR)
- :"dIr" (nr));
+ :"dIr" (nr), "m" (ADDR));
}
/**
@@ -55,7 +55,7 @@ static __inline__ void __set_bit(long nr
__asm__(
"btsq %1,%0"
:"=m" (ADDR)
- :"dIr" (nr));
+ :"dIr" (nr), "m" (ADDR));
}
/**
@@ -73,7 +73,7 @@ static __inline__ void clear_bit(long nr
__asm__ __volatile__( LOCK_PREFIX
"btrq %1,%0"
:"=m" (ADDR)
- :"dIr" (nr));
+ :"dIr" (nr), "m" (ADDR));
}
#define smp_mb__before_clear_bit() barrier()
#define smp_mb__after_clear_bit() barrier()
@@ -92,7 +92,7 @@ static __inline__ void __change_bit(long
__asm__ __volatile__(
"btcq %1,%0"
:"=m" (ADDR)
- :"dIr" (nr));
+ :"dIr" (nr), "m" (ADDR));
}
/**
@@ -109,7 +109,7 @@ static __inline__ void change_bit(long n
__asm__ __volatile__( LOCK_PREFIX
"btcq %1,%0"
:"=m" (ADDR)
- :"dIr" (nr));
+ :"dIr" (nr), "m" (ADDR));
}
/**
@@ -127,7 +127,7 @@ static __inline__ int test_and_set_bit(l
__asm__ __volatile__( LOCK_PREFIX
"btsq %2,%1\n\tsbbq %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"dIr" (nr) : "memory");
+ :"dIr" (nr), "m" (ADDR) : "memory");
return oldbit;
}
@@ -147,7 +147,7 @@ static __inline__ int __test_and_set_bit
__asm__(
"btsq %2,%1\n\tsbbq %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"dIr" (nr));
+ :"dIr" (nr), "m" (ADDR));
return oldbit;
}
@@ -166,7 +166,7 @@ static __inline__ int test_and_clear_bit
__asm__ __volatile__( LOCK_PREFIX
"btrq %2,%1\n\tsbbq %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"dIr" (nr) : "memory");
+ :"dIr" (nr), "m" (ADDR) : "memory");
return oldbit;
}
@@ -186,7 +186,7 @@ static __inline__ int __test_and_clear_b
__asm__(
"btrq %2,%1\n\tsbbq %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"dIr" (nr));
+ :"dIr" (nr), "m" (ADDR));
return oldbit;
}
@@ -198,7 +198,7 @@ static __inline__ int __test_and_change_
__asm__ __volatile__(
"btcq %2,%1\n\tsbbq %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"dIr" (nr) : "memory");
+ :"dIr" (nr), "m" (ADDR) : "memory");
return oldbit;
}
@@ -217,7 +217,7 @@ static __inline__ int test_and_change_bi
__asm__ __volatile__( LOCK_PREFIX
"btcq %2,%1\n\tsbbq %0,%0"
:"=r" (oldbit),"=m" (ADDR)
- :"dIr" (nr) : "memory");
+ :"dIr" (nr), "m" (ADDR) : "memory");
return oldbit;
}
--
1.4.1
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
