Re: [patch] i386: make bitops safe
From: Chuck Ebbert
Date: Tue Feb 28 2006 - 01:05:25 EST
In-Reply-To: <20060228005436.GA24895@xxxxxxxxxx>
On Mon, 27 Feb 2006 at 16:54:36 -0800, Richard Henderson wrote:
> On Tue, Feb 28, 2006 at 12:47:22AM +0100, Andi Kleen wrote:
> > I remember asking rth about this at some point and IIRC
> > he expressed doubts if it would actually do what expected. Richard?
>
> It's a bit dicey to be sure. GCC may or may not be able to look
> through the size of the array and not kill things beyond it. If
> one could be *sure* of some actual maximum index, this would be
> fine, but I don't think you can.
>
In theory the bit offset could be from -2**31 to 2**31 - 1
> One could reasonably argue that if you used a structure with a
> flexible array member, that GCC could not look through that. But
> again I'm not 100% positive this is handled properly.
This seems to work but causes more problems than it solves:
#define vaddr ((volatile long *) addr)
static inline void set_bit(int nr, volatile unsigned long * addr)
{
__asm__ __volatile__( "lock ; "
"btsl %2,%1"
:"+m" (*(vaddr + (nr>>5)))
:"m" (*vaddr),"Ir" (nr)
);
}
First, it generates the byte offset nr>>5 and puts it in a register
even though it will never be used in the asm. I can't find a constraint
that says "I'll be accessing this address but I don't need you to generate
it for me." Second, the compiler thinks *vaddr will be read when it
really won't (unless nr>>5 == 0 in which case constraint 0 takes care
of it.)
Generated code when nr is a variable:
movl nr,%edx
movl %edx,%eax
sarl $5,%eax
sall $2,%eax
lock ; btsl %edx,addr
This causes a register reload afterward (assuming all regs are busy) and
can cause a function to use more stack space. That plus the three extra
instructions made me go with the full memory clobber instead.
--
Chuck
"Equations are the Devil's sentences." --Stephen Colbert
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/