Re: [patch 3/8] mutex subsystem, add atomic_*_call_if_*() to i386

Linus Torvalds
2005-12-21T18:56:44Z

On Wed, 21 Dec 2005, Ingo Molnar wrote:
>
> add two new atomic ops to i386: atomic_dec_call_if_negative() and
> atomic_inc_call_if_nonpositive(), which are conditional-call-if
> atomic operations. Needed by the new mutex code.

Umm. This asm is broken. It doesn't mark %eax as changed, so this is only
reliable if the function you call is

- a "fastcall" one
- always returns as its return value the pointer to the atomic count

which is not true (you verify that it's a fastcall, but it's of type
"void").

Now, it's entirely possible that it's only used in situations where the
compiler doesn't rely on the value of %eax after the asm anyway, but
that's just praying. Either mark the value as being changed, or just
save/restore the registers. Ie either something like

__asm__ __volatile__(
LOCK "decl (%0)\n\t"
"js 2f\n"
"1:\n"
LOCK_SECTION_START("")
"2: call "#fn_name"\n\t"
"jmp 1b\n"
LOCK_SECTION_END
:"=a" (dummy)
:"0" (v)
:"memory","cx","dx");

or just do

__asm__ __volatile__(
LOCK "decl (%0)\n\t"
"js 2f\n"
"1:\n"
LOCK_SECTION_START("")
"pushl %%ebx\n\t"
"pushl %%ecx\n\t"
"pushl %%eax\n\t"
"call "#fn_name"\n\t"
"popl %%eax\n\t"
"popl %%ecx\n\t"
"popl %%ebx\n\t"
"jmp 1b\n"
LOCK_SECTION_END
:/* no outputs */
:"a" (v)
:"memory");

or some in-between thing (that saves only part of the registers).

The above has not been tested in any way, shape or form. But you get the idea.

Linus