From: Nick Piggin <nickpiggin@yahoo.com.au>
To: David Howells <dhowells@redhat.com>
Cc: Dominik Brodowski <linux@dominikbrodowski.de>,
linux-kernel@vger.kernel.org
Subject: Re: [RFC][PATCH 2/4] interruptible rwsem operations (i386, core)
Date: Thu, 20 Jan 2005 12:44:05 +1100 [thread overview]
Message-ID: <41EF0CE5.7080305@yahoo.com.au> (raw)
In-Reply-To: <24595.1106176220@redhat.com>
David Howells wrote:
> Dominik Brodowski <linux@dominikbrodowski.de> wrote:
>
>
>>Add functions down_read_interruptible, and down_write_interruptible to rw
>>semaphores. Implement these for i386.
>>...
>
>
>>+static inline int
>>+rwsem_down_interruptible_failed_common(struct rw_semaphore *sem,
>>+ struct rwsem_waiter *waiter, signed long adjustment)
>>+{
>>...
>
>
> I wonder if you should check to see if there are any readers that can be woken
> up if a sleeping writer is interrupted, but I can't think of a simple way to
> do it.
>
>
I think it will, won't it?
>>-struct rw_semaphore fastcall __sched *
>>-rwsem_down_read_failed(struct rw_semaphore *sem)
>>+void fastcall __sched rwsem_down_read_failed(struct rw_semaphore *sem)
>
>
> Please don't.
>
>
>>@@ -199,14 +253,33 @@ rwsem_down_read_failed(struct rw_semapho
>> RWSEM_WAITING_BIAS - RWSEM_ACTIVE_BIAS);
>>
>> rwsemtrace(sem, "Leaving rwsem_down_read_failed");
>>- return sem;
>
>
> Ditto.
>
>
>>-struct rw_semaphore fastcall __sched *
>>-rwsem_down_write_failed(struct rw_semaphore *sem)
>>+void fastcall __sched rwsem_down_write_failed(struct rw_semaphore *sem)
>
>
> Ditto.
>
>
>>@@ -216,10 +289,31 @@ rwsem_down_write_failed(struct rw_semaph
>> rwsem_down_failed_common(sem, &waiter, -RWSEM_ACTIVE_BIAS);
>>
>> rwsemtrace(sem, "Leaving rwsem_down_write_failed");
>>- return sem;
>
>
> Ditto.
>
>
>>@@ -99,11 +103,12 @@ static inline void __down_read(struct rw
>> {
>> __asm__ __volatile__(
>> "# beginning down_read\n\t"
>>-LOCK_PREFIX " incl (%%eax)\n\t" /* adds 0x00000001, returns the old value */
>>+LOCK_PREFIX " incl %0\n\t" /* adds 0x00000001, returns the old value */
>
>
> Ditto.
>
>
>> " js 2f\n\t" /* jump if we weren't granted the lock */
>> "1:\n\t"
>> LOCK_SECTION_START("")
>> "2:\n\t"
>>+ " movl %2,%%eax\n\t"
>
>
> Splat.
>
>
>> " pushl %%ecx\n\t"
>> " pushl %%edx\n\t"
>> " call rwsem_down_read_failed\n\t"
>
>
> Splat.
>
>
>>@@ -113,11 +118,41 @@ LOCK_PREFIX " incl (%%eax)\n\t" /*
>> LOCK_SECTION_END
>> "# ending down_read\n\t"
>> : "=m"(sem->count)
>>- : "a"(sem), "m"(sem->count)
>>+ : "m"(sem->count), "m"(sem)
>> : "memory", "cc");
>> }
>
>
> You appear to be corrupting EAX.
>
>
>>+static inline int __down_read_interruptible(struct rw_semaphore *sem)
>
>
> Will corrupt EAX.
>
I'll fix these up. You're right by the looks.
>
>> "# beginning down_write\n\t"
>>-LOCK_PREFIX " xadd %%edx,(%%eax)\n\t" /* subtract 0x0000ffff, returns the old value */
>>+LOCK_PREFIX " xadd %%edx,%0\n\t" /* subtract 0x0000ffff, returns the old value */
>
>
> Again, please don't. It's a lot more readable when it mentions EAX directly,
> plus it's also independent of constraint reordering.
>
OK I suppose so...
prev parent reply other threads:[~2005-01-20 1:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-01-19 21:38 [RFC][PATCH 2/4] interruptible rwsem operations (i386, core) Dominik Brodowski
2005-01-19 23:10 ` David Howells
2005-01-20 1:44 ` Nick Piggin [this message]
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=41EF0CE5.7080305@yahoo.com.au \
--to=nickpiggin@yahoo.com.au \
--cc=dhowells@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@dominikbrodowski.de \
/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).