[PATCH][1/2] fix for -mm add-sem_is_read-write_locked.patch

[PATCH][1/2] fix for -mm add-sem_is_read-write_locked.patch

[Rik van Riel]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6099 bytes --]

Hi Andrew,

Here is an incremental fix to the add-sem_is_read-write_locked
patch in -mm.  Also attached is a full version of that file,
which can just be dropped into place - I've verified that none
of the patches in your stack get rejects.

The reason for this change is that a lock that's held for
reading can be negative when there is a writer waiting, as
David pointed out to me a while ago.

The corresponding change to swaptoken-tuning.patch is in the
next mail.

Signed-off-by: Rik van Riel <riel@redhat.com>

Index: linux-2.6.13/include/asm-alpha/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-alpha/rwsem.h
+++ linux-2.6.13/include/asm-alpha/rwsem.h
@@ -262,14 +262,9 @@ static inline long rwsem_atomic_update(l
 #endif
 }
 
-static inline int sem_is_read_locked(struct rw_semaphore *sem)
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return (sem->count > 0);
-}
-
-static inline int sem_is_write_locked(struct rw_semaphore *sem)
-{
-	return (sem->count < 0);
+	return (sem->count != 0);
 }
 
 #endif /* __KERNEL__ */
Index: linux-2.6.13/include/asm-i386/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-i386/rwsem.h
+++ linux-2.6.13/include/asm-i386/rwsem.h
@@ -284,14 +284,9 @@ LOCK_PREFIX	"xadd %0,(%2)"
 	return tmp+delta;
 }
 
-static inline int sem_is_read_locked(struct rw_semaphore *sem)
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return (sem->count > 0);
-}
-
-static inline int sem_is_write_locked(struct rw_semaphore *sem)
-{
-	return (sem->count < 0);
+	return (sem->count != 0);
 }
 
 #endif /* __KERNEL__ */
Index: linux-2.6.13/include/asm-ia64/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-ia64/rwsem.h
+++ linux-2.6.13/include/asm-ia64/rwsem.h
@@ -186,14 +186,9 @@ __downgrade_write (struct rw_semaphore *
 #define rwsem_atomic_add(delta, sem)	atomic64_add(delta, (atomic64_t *)(&(sem)->count))
 #define rwsem_atomic_update(delta, sem)	atomic64_add_return(delta, (atomic64_t *)(&(sem)->count))
 
-static inline int sem_is_read_locked(struct rw_semaphore *sem)
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return (sem->count > 0);
-}
-
-static inline int sem_is_write_locked(struct rw_semaphore *sem)
-{
-	return (sem->count < 0);
+	return (sem->count != 0);
 }
 
 #endif /* _ASM_IA64_RWSEM_H */
Index: linux-2.6.13/include/asm-ppc64/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-ppc64/rwsem.h
+++ linux-2.6.13/include/asm-ppc64/rwsem.h
@@ -165,12 +165,7 @@ static inline int rwsem_atomic_update(in
 
 static inline int sem_is_read_locked(struct rw_semaphore *sem)
 {
-	return (sem->count > 0);
-}
-
-static inline int sem_is_write_locked(struct rw_semaphore *sem)
-{
-	return (sem->count < 0);
+	return (sem->count != 0);
 }
 
 #endif /* __KERNEL__ */
Index: linux-2.6.13/include/asm-ppc/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-ppc/rwsem.h
+++ linux-2.6.13/include/asm-ppc/rwsem.h
@@ -168,14 +168,9 @@ static inline int rwsem_atomic_update(in
 	return atomic_add_return(delta, (atomic_t *)(&sem->count));
 }
 
-static inline int sem_is_read_locked(struct rw_semaphore *sem)
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return (sem->count > 0);
-}
-
-static inline int sem_is_write_locked(struct rw_semaphore *sem)
-{
-	return (sem->count < 0);
+	return (sem->count != 0);
 }
 
 #endif /* __KERNEL__ */
Index: linux-2.6.13/include/asm-s390/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-s390/rwsem.h
+++ linux-2.6.13/include/asm-s390/rwsem.h
@@ -351,14 +351,9 @@ static inline long rwsem_atomic_update(l
 	return new;
 }
 
-static inline int sem_is_read_locked(struct rw_semaphore *sem)
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return (sem->count > 0);
-}
-
-static inline int sem_is_write_locked(struct rw_semaphore *sem)
-{
-	return (sem->count < 0);
+	return (sem->count != 0);
 }
 
 #endif /* __KERNEL__ */
Index: linux-2.6.13/include/asm-sh/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-sh/rwsem.h
+++ linux-2.6.13/include/asm-sh/rwsem.h
@@ -166,14 +166,9 @@ static inline int rwsem_atomic_update(in
 	return atomic_add_return(delta, (atomic_t *)(&sem->count));
 }
 
-static inline int sem_is_read_locked(struct rw_semaphore *sem)
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return (sem->count > 0);
-}
-
-static inline int sem_is_write_locked(struct rw_semaphore *sem)
-{
-	return (sem->count < 0);
+	return (sem->count != 0);
 }
 
 #endif /* __KERNEL__ */
Index: linux-2.6.13/include/asm-sparc64/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-sparc64/rwsem.h
+++ linux-2.6.13/include/asm-sparc64/rwsem.h
@@ -56,14 +56,9 @@ static inline void rwsem_atomic_add(int 
 	atomic_add(delta, (atomic_t *)(&sem->count));
 }
 
-static inline int sem_is_read_locked(struct rw_semaphore *sem)
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return (sem->count > 0);
-}
-
-static inline int sem_is_write_locked(struct rw_semaphore *sem)
-{
-	return (sem->count < 0);
+	return (sem->count != 0);
 }
 
 #endif /* __KERNEL__ */
Index: linux-2.6.13/include/asm-x86_64/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-x86_64/rwsem.h
+++ linux-2.6.13/include/asm-x86_64/rwsem.h
@@ -274,14 +274,9 @@ LOCK_PREFIX	"xaddl %0,(%2)"
 	return tmp+delta;
 }
 
-static inline int sem_is_read_locked(struct rw_semaphore *sem)
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
-	return (sem->count > 0);
-}
-
-static inline int sem_is_write_locked(struct rw_semaphore *sem)
-{
-	return (sem->count < 0);
+	return (sem->count != 0);
 }
 
 #endif /* __KERNEL__ */

[-- Attachment #2: Type: TEXT/PLAIN, Size: 5437 bytes --]


From: Rik Van Riel <riel@redhat.com>

Add sem_is_read/write_locked functions to the read/write semaphores, along the
same lines of the *_is_locked spinlock functions.  The swap token tuning patch
uses sem_is_read_locked; sem_is_write_locked is added for completeness.

Signed-off-by: Rik van Riel <riel@redhat.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Index: linux-2.6.13/include/asm-alpha/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-alpha/rwsem.h
+++ linux-2.6.13/include/asm-alpha/rwsem.h
@@ -262,5 +262,10 @@ static inline long rwsem_atomic_update(l
 #endif
 }
 
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
+{
+	return (sem->count != 0);
+}
+
 #endif /* __KERNEL__ */
 #endif /* _ALPHA_RWSEM_H */
Index: linux-2.6.13/include/asm-i386/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-i386/rwsem.h
+++ linux-2.6.13/include/asm-i386/rwsem.h
@@ -284,5 +284,10 @@ LOCK_PREFIX	"xadd %0,(%2)"
 	return tmp+delta;
 }
 
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
+{
+	return (sem->count != 0);
+}
+
 #endif /* __KERNEL__ */
 #endif /* _I386_RWSEM_H */
Index: linux-2.6.13/include/asm-ia64/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-ia64/rwsem.h
+++ linux-2.6.13/include/asm-ia64/rwsem.h
@@ -186,4 +186,9 @@ __downgrade_write (struct rw_semaphore *
 #define rwsem_atomic_add(delta, sem)	atomic64_add(delta, (atomic64_t *)(&(sem)->count))
 #define rwsem_atomic_update(delta, sem)	atomic64_add_return(delta, (atomic64_t *)(&(sem)->count))
 
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
+{
+	return (sem->count != 0);
+}
+
 #endif /* _ASM_IA64_RWSEM_H */
Index: linux-2.6.13/include/asm-ppc64/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-ppc64/rwsem.h
+++ linux-2.6.13/include/asm-ppc64/rwsem.h
@@ -163,5 +163,10 @@ static inline int rwsem_atomic_update(in
 	return atomic_add_return(delta, (atomic_t *)(&sem->count));
 }
 
+static inline int sem_is_read_locked(struct rw_semaphore *sem)
+{
+	return (sem->count != 0);
+}
+
 #endif /* __KERNEL__ */
 #endif /* _PPC_RWSEM_XADD_H */
Index: linux-2.6.13/include/asm-ppc/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-ppc/rwsem.h
+++ linux-2.6.13/include/asm-ppc/rwsem.h
@@ -168,5 +168,10 @@ static inline int rwsem_atomic_update(in
 	return atomic_add_return(delta, (atomic_t *)(&sem->count));
 }
 
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
+{
+	return (sem->count != 0);
+}
+
 #endif /* __KERNEL__ */
 #endif /* _PPC_RWSEM_XADD_H */
Index: linux-2.6.13/include/asm-s390/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-s390/rwsem.h
+++ linux-2.6.13/include/asm-s390/rwsem.h
@@ -351,5 +351,10 @@ static inline long rwsem_atomic_update(l
 	return new;
 }
 
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
+{
+	return (sem->count != 0);
+}
+
 #endif /* __KERNEL__ */
 #endif /* _S390_RWSEM_H */
Index: linux-2.6.13/include/asm-sh/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-sh/rwsem.h
+++ linux-2.6.13/include/asm-sh/rwsem.h
@@ -166,5 +166,10 @@ static inline int rwsem_atomic_update(in
 	return atomic_add_return(delta, (atomic_t *)(&sem->count));
 }
 
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
+{
+	return (sem->count != 0);
+}
+
 #endif /* __KERNEL__ */
 #endif /* _ASM_SH_RWSEM_H */
Index: linux-2.6.13/include/asm-sparc64/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-sparc64/rwsem.h
+++ linux-2.6.13/include/asm-sparc64/rwsem.h
@@ -56,6 +56,11 @@ static inline void rwsem_atomic_add(int 
 	atomic_add(delta, (atomic_t *)(&sem->count));
 }
 
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
+{
+	return (sem->count != 0);
+}
+
 #endif /* __KERNEL__ */
 
 #endif /* _SPARC64_RWSEM_H */
Index: linux-2.6.13/include/asm-x86_64/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-x86_64/rwsem.h
+++ linux-2.6.13/include/asm-x86_64/rwsem.h
@@ -274,5 +274,10 @@ LOCK_PREFIX	"xaddl %0,(%2)"
 	return tmp+delta;
 }
 
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
+{
+	return (sem->count != 0);
+}
+
 #endif /* __KERNEL__ */
 #endif /* _X8664_RWSEM_H */
Index: linux-2.6.13/include/linux/rwsem-spinlock.h
===================================================================
--- linux-2.6.13.orig/include/linux/rwsem-spinlock.h
+++ linux-2.6.13/include/linux/rwsem-spinlock.h
@@ -61,5 +61,15 @@ extern void FASTCALL(__up_read(struct rw
 extern void FASTCALL(__up_write(struct rw_semaphore *sem));
 extern void FASTCALL(__downgrade_write(struct rw_semaphore *sem));
 
+static inline int sem_is_read_locked(struct rw_semaphore *sem)
+{
+	return (sem->activity > 0);
+}
+
+static inline int sem_is_write_locked(struct rw_semaphore *sem)
+{
+	return (sem->activity < 0);
+}
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_RWSEM_SPINLOCK_H */

Re: [PATCH][1/2] fix for -mm add-sem_is_read-write_locked.patch

[David Howells]

Rik van Riel <riel@redhat.com> wrote:

> Here is an incremental fix to the add-sem_is_read-write_locked
> patch in -mm.  Also attached is a full version of that file,
> which can just be dropped into place - I've verified that none
> of the patches in your stack get rejects.

The comment attached to the drop-in replacement patch is wrong:

| [1. text/plain; add-sem_is_read-write_locked.patch]   
| 
| From: Rik Van Riel <riel@redhat.com>
| 
| Add sem_is_read/write_locked functions to the read/write semaphores, along the
| same lines of the *_is_locked spinlock functions.  The swap token tuning patch
| uses sem_is_read_locked; sem_is_write_locked is added for completeness.

The function names you've used are incorrect.

Furthermore, the substance of the patch is wrong in a number of ways:

| Index: linux-2.6.13/include/asm-ppc64/rwsem.h
| ===================================================================
| --- linux-2.6.13.orig/include/asm-ppc64/rwsem.h
| +++ linux-2.6.13/include/asm-ppc64/rwsem.h
| @@ -163,5 +163,10 @@ static inline int rwsem_atomic_update(in
|  	return atomic_add_return(delta, (atomic_t *)(&sem->count));
|  }
|  
| +static inline int sem_is_read_locked(struct rw_semaphore *sem)
| +{
| +	return (sem->count != 0);
| +}
| +

This uses the function wrong name. And:

| Index: linux-2.6.13/include/linux/rwsem-spinlock.h
| ===================================================================
| --- linux-2.6.13.orig/include/linux/rwsem-spinlock.h
| +++ linux-2.6.13/include/linux/rwsem-spinlock.h
| @@ -61,5 +61,15 @@ extern void FASTCALL(__up_read(struct rw
|  extern void FASTCALL(__up_write(struct rw_semaphore *sem));
|  extern void FASTCALL(__downgrade_write(struct rw_semaphore *sem));
|  
| +static inline int sem_is_read_locked(struct rw_semaphore *sem)
| +{
| +	return (sem->activity > 0);
| +}
| +
| +static inline int sem_is_write_locked(struct rw_semaphore *sem)
| +{
| +	return (sem->activity < 0);
| +}
| +

Is inconsistent, though the tests are valid.

Also, you don't need to bracket the expression handed to the return directive,
but that's a minor matter.

David

Re: [PATCH][1/2] fix for -mm add-sem_is_read-write_locked.patch

[Rik van Riel]

On Mon, 5 Sep 2005, David Howells wrote:

> The comment attached to the drop-in replacement patch is wrong:

Indeed it is.  Good thing Andrew doesn't seem to have dropped
it in ;)

> | +static inline int sem_is_read_locked(struct rw_semaphore *sem)
> | +{
> | +	return (sem->count != 0);
> | +}
> | +
> 
> This uses the function wrong name. And:

Argh.  That should be rwsem_is_locked of course...

> Is inconsistent, though the tests are valid.

I fixed that one in a separate patch.

-- 
All Rights Reversed

Re: [PATCH][1/2] fix for -mm add-sem_is_read-write_locked.patch

[Rik van Riel]

On Mon, 5 Sep 2005, David Howells wrote:

> | Index: linux-2.6.13/include/asm-ppc64/rwsem.h

> This uses the function wrong name. And:

I really shouldn't make patches like this in the morning on weekends, 
when I'm still sleepy.  Well, here is the trivial fix...

Signed-off-by: Rik van Riel <riel@redhat.com>

Index: linux-2.6.13/include/asm-ppc64/rwsem.h
===================================================================
--- linux-2.6.13.orig/include/asm-ppc64/rwsem.h
+++ linux-2.6.13/include/asm-ppc64/rwsem.h
@@ -163,7 +163,7 @@ static inline int rwsem_atomic_update(in
 	return atomic_add_return(delta, (atomic_t *)(&sem->count));
 }
 
-static inline int sem_is_read_locked(struct rw_semaphore *sem)
+static inline int rwsem_is_locked(struct rw_semaphore *sem)
 {
 	return (sem->count != 0);
 }

Re: [PATCH][1/2] fix for -mm add-sem_is_read-write_locked.patch

[David Howells]

Rik van Riel <riel@redhat.com> wrote:

> I really shouldn't make patches like this in the morning on weekends, 
> when I'm still sleepy.  Well, here is the trivial fix...

That would seem to fix that bit.

David