From: Linus Torvalds <torvalds@osdl.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: lkml <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@osdl.org>,
Arjan van de Ven <arjanv@infradead.org>,
Jes Sorensen <jes@trained-monkey.org>,
Zwane Mwaikambo <zwane@arm.linux.org.uk>,
Oleg Nesterov <oleg@tv-sign.ru>,
David Howells <dhowells@redhat.com>,
Alan Cox <alan@lxorguk.ukuu.org.uk>,
Benjamin LaHaise <bcrl@kvack.org>,
Steven Rostedt <rostedt@goodmis.org>,
Christoph Hellwig <hch@infradead.org>, Andi Kleen <ak@suse.de>,
Russell King <rmk+lkml@arm.linux.org.uk>,
Nicolas Pitre <nico@cam.org>
Subject: Re: [patch 3/8] mutex subsystem, add atomic_*_call_if_*() to i386
Date: Wed, 21 Dec 2005 10:54:20 -0800 (PST) [thread overview]
Message-ID: <Pine.LNX.4.64.0512211044240.4827@g5.osdl.org> (raw)
In-Reply-To: <20051221155442.GD7243@elte.hu>
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
next prev parent reply other threads:[~2005-12-21 18:56 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-12-21 15:54 [patch 3/8] mutex subsystem, add atomic_*_call_if_*() to i386 Ingo Molnar
2005-12-21 18:54 ` Linus Torvalds [this message]
2005-12-21 18:57 ` Linus Torvalds
2005-12-21 20:01 ` Ingo Molnar
2005-12-21 20:32 ` Daniel Jacobowitz
2005-12-21 20:54 ` Nicolas Pitre
2005-12-22 9:18 ` Keith Owens
2005-12-21 19:47 ` Ingo Molnar
2005-12-21 22:40 ` Linus Torvalds
2005-12-21 22:37 Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Pine.LNX.4.64.0512211044240.4827@g5.osdl.org \
--to=torvalds@osdl.org \
--cc=ak@suse.de \
--cc=akpm@osdl.org \
--cc=alan@lxorguk.ukuu.org.uk \
--cc=arjanv@infradead.org \
--cc=bcrl@kvack.org \
--cc=dhowells@redhat.com \
--cc=hch@infradead.org \
--cc=jes@trained-monkey.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=nico@cam.org \
--cc=oleg@tv-sign.ru \
--cc=rmk+lkml@arm.linux.org.uk \
--cc=rostedt@goodmis.org \
--cc=zwane@arm.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).