lkml.org 
[lkml]   [2005]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC][PATCH 2/4] interruptible rwsem operations (i386, core)
Date

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.

> -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.

> "# 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.

> +static inline int __down_write_interruptible(struct rw_semaphore *sem)

Will corrupt EAX and EDX.

David
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:09    [W:0.062 / U:0.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site