lkml.org 
[lkml]   [2010]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: x86-32: clean up rwsem inline asm statements
On 01/13/2010 11:58 AM, George Spelvin wrote:
>> As far as I can tell, very few of these assembly statements actually
>> need a size at all -- the very first inc statement is purely to memory,
>> and as such it needs a size marker, but almost any operation which is
>> register-register or register-memory will simply take its size from the
>> register operand. For those, it seems cleaner to simply drop the size
>> suffix, and in fact this is the style we have been pushing people
>> towards (use the suffix where mandatory or where the size is fixed
>> anyway, to help catch bugs; use no suffix where the size can vary and is
>> implied by the operands.)
>
> The one thing is that for a register-memory operation, using the
> size of the memory operand can catch bugs where it doesn't match
> the size of the register operand.
>
> GCC's inline asm doesn't make operand size very implicit, and it's
> awkward to cast output operands, so there's a potential for bugs.
> I especially get nervous when the operand itself is an immediate
> constant, as I can't remember the ANSI rules for the type very well.
> (Quick: is 0x80000000 an unsigned 32-bit int or a signed 64-bit one?
> What about 2147483648 or 1<<31?)
>
> Since this is mostly inline functions, it's not so big a problem, but
> I'd consider something like:
>
> static inline void __up_write(struct rw_semaphore *sem)
> {
> unsigned long tmp;
> asm volatile("# beginning __up_write\n\t"
> LOCK_PREFIX " xadd%z0 %1,(%2)\n\t"
> /* tries to transition
> 0xffff0001 -> 0x00000000 */
> " jz 1f\n"
> " call call_rwsem_wake\n"
> "1:\n\t"
> "# ending __up_write\n"
> : "+m" (sem->count), "=d" (tmp)
> : "a" (sem), "1" (-RWSEM_ACTIVE_WRITE_BIAS)
> : "memory", "cc");
> }
>
> Just in case the size of -RWSEM_ACTIVE_WRITE_BIAS doesn't match that
> of sem->count. It'll explode when you try to run it, of course, but
> there's something to be said for compile-time errors.

There are a number of things that can be done better... for one thing,
"+m" (sem->count) and "a" (sem) is just bloody wrong. The right thing
would be "a" (&sem->count) for proper robustness.

There is no real point in being concerned about the type of immediates,
because the immediate type isn't really used... it shows up as a literal
in the assembly language. However, if you're really concerned, the
right thing to do is to do a cast in C, not playing games with the assembly.

-hpa


\
 
 \ /
  Last update: 2010-01-13 22:07    [W:0.035 / U:0.188 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site