[patch] Add a writer prior lock methord for rwlock

[patch] Add a writer prior lock methord for rwlock

[Liu Tao]

The patch adds the write_forcelock() methord, which has higher priority than
read_lock() and write_lock(). The original read_lock() and write_lock() 
is not
touched, and the unlock methord is still write_unlock();
The patch gives implemention on i386, for kernel 2.6.7.

Regards

-----------------------

diff -Naur linux-2.6.7/include/asm-i386/rwlock.h 
linux-2.6.7-dev/include/asm-i386/rwlock.h
--- linux-2.6.7/include/asm-i386/rwlock.h    2004-06-16 
13:20:04.000000000 +0800
+++ linux-2.6.7-dev/include/asm-i386/rwlock.h    2004-08-03 
09:15:02.000000000 +0800
@@ -80,4 +80,37 @@
                             __build_write_lock_ptr(rw, helper); \
                     } while (0)
 
+#define __build_write_forcelock_ptr(rw) \
+    asm volatile(LOCK "subl $" RW_LOCK_BIAS_STR ",(%0)\n\t" \
+             "jnz 2f\n" \
+             "1:\n" \
+             LOCK_SECTION_START("") \
+             "2:\t" \
+             "rep;nop\n\t" \
+             "cmpl $0,(%0)\n\t" \
+             "jnz 2b\n\t" \
+             "jmp 1b\n" \
+             LOCK_SECTION_END \
+             ::"a" (rw) : "memory")
+
+#define __build_write_forcelock_const(rw) \
+    asm volatile(LOCK "subl $" RW_LOCK_BIAS_STR ",%0\n\t" \
+             "jnz 2f\n" \
+             "1:\n" \
+             LOCK_SECTION_START("") \
+             "2:\t" \
+             "rep;nop\n\t" \
+             "cmpl $0,%0\n\t" \
+             "jnz 2b\n\t" \
+             "jmp 1b\n" \
+             LOCK_SECTION_END \
+             :"=m" (*(volatile int *)rw) : : "memory")
+
+#define __build_write_forcelock(rw)    do { \
+                        if (__builtin_constant_p(rw)) \
+                            __build_write_forcelock_const(rw); \
+                        else \
+                            __build_write_forcelock_ptr(rw); \
+                    } while (0)
+                             
 #endif
diff -Naur linux-2.6.7/include/asm-i386/spinlock.h 
linux-2.6.7-dev/include/asm-i386/spinlock.h
--- linux-2.6.7/include/asm-i386/spinlock.h    2004-06-16 
13:19:02.000000000 +0800
+++ linux-2.6.7-dev/include/asm-i386/spinlock.h    2004-08-02 
16:42:26.000000000 +0800
@@ -139,11 +139,13 @@
  */
 typedef struct {
     volatile unsigned int lock;
+    spinlock_t forcelock;
 #ifdef CONFIG_DEBUG_SPINLOCK
     unsigned magic;
 #endif
 } rwlock_t;
 
+#define RWLOCK_FORCELOCK    , {1 SPINLOCK_MAGIC_INIT}
 #define RWLOCK_MAGIC    0xdeaf1eed
 
 #ifdef CONFIG_DEBUG_SPINLOCK
@@ -152,7 +154,7 @@
 #define RWLOCK_MAGIC_INIT    /* */
 #endif
 
-#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_MAGIC_INIT }
+#define RW_LOCK_UNLOCKED (rwlock_t) { RW_LOCK_BIAS RWLOCK_FORCELOCK 
RWLOCK_MAGIC_INIT }
 
 #define rwlock_init(x)    do { *(x) = RW_LOCK_UNLOCKED; } while(0)
 
@@ -185,6 +187,16 @@
     __build_write_lock(rw, "__write_lock_failed");
 }
 
+static inline void _raw_write_forcelock(rwlock_t *rw)
+{
+#ifdef CONFIG_DEBUG_SPINLOCK
+    BUG_ON(rw->magic != RWLOCK_MAGIC);
+#endif
+    _raw_spin_lock(&rw->forcelock);
+    __build_write_forcelock(rw);
+    _raw_spin_unlock(&rw->forcelock);
+}
+
 #define _raw_read_unlock(rw)        asm volatile("lock ; incl %0" :"=m" 
((rw)->lock) : : "memory")
 #define _raw_write_unlock(rw)    asm volatile("lock ; addl $" 
RW_LOCK_BIAS_STR ",%0":"=m" ((rw)->lock) : : "memory")
 
diff -Naur linux-2.6.7/include/linux/spinlock.h 
linux-2.6.7-dev/include/linux/spinlock.h
--- linux-2.6.7/include/linux/spinlock.h    2004-06-16 
13:19:23.000000000 +0800
+++ linux-2.6.7-dev/include/linux/spinlock.h    2004-08-04 
16:25:37.000000000 +0800
@@ -231,6 +231,12 @@
 } while(0)
 #endif
 
+#define write_forcelock(lock) \
+do { \
+    preempt_disable(); \
+    _raw_write_forcelock(lock); \
+} while (0)
+
 #define read_lock(lock)    \
 do { \
     preempt_disable(); \
@@ -318,6 +324,27 @@
     _raw_write_lock(lock); \
 } while (0)
 
+#define write_forcelock_irqsave(lock, flags) \
+do { \
+    local_irq_save(flags); \
+    preempt_disable(); \
+    _raw_write_forcelock(lock); \
+} while (0)
+
+#define write_forcelock_irq(lock) \
+do { \
+    local_irq_disable(); \
+    preempt_disable(); \
+    _raw_write_forcelock(lock); \
+} while (0)
+
+#define write_forcelock_bh(lock) \
+do { \
+    local_bh_disable(); \
+    preempt_disable(); \
+    _raw_write_forcelock(lock); \
+} while (0)
+
 #define spin_unlock_irqrestore(lock, flags) \
 do { \
     _raw_spin_unlock(lock); \

Re: [patch] Add a writer prior lock methord for rwlock

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 456 bytes --]

On Wed, 2004-08-04 at 11:09, Liu Tao wrote:
> The patch adds the write_forcelock() methord, which has higher priority than
> read_lock() and write_lock(). The original read_lock() and write_lock() 
> is not
> touched, and the unlock methord is still write_unlock();
> The patch gives implemention on i386, for kernel 2.6.7.

can you please elaborate what kind of usage scenarios this would be
useful ? It would be nice to know what this is for ;)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch] Add a writer prior lock methord for rwlock

[Oliver Neukum]

Am Mittwoch, 4. August 2004 11:09 schrieb Liu Tao:
> The patch adds the write_forcelock() methord, which has higher priority than
> read_lock() and write_lock(). The original read_lock() and write_lock() 
> is not
> touched, and the unlock methord is still write_unlock();

It seems to me that with this a recursive read_lock() with
a read lock already held may deadlock.

	Regards
		Oliver

Re: [patch] Add a writer prior lock methord for rwlock

[Liu Tao]

Arjan van de Ven wrote:

>On Wed, 2004-08-04 at 11:09, Liu Tao wrote:
>  
>
>>The patch adds the write_forcelock() methord, which has higher priority than
>>read_lock() and write_lock(). The original read_lock() and write_lock() 
>>is not
>>touched, and the unlock methord is still write_unlock();
>>The patch gives implemention on i386, for kernel 2.6.7.
>>    
>>
>
>can you please elaborate what kind of usage scenarios this would be
>useful ? It would be nice to know what this is for ;)
>  
>

write_forcelock() should be used to avoid readers starve writers, or for 
writers
to update shared data as far as possiable, since it prevents new readers 
acquire
the lock while it's waiting for existing readers release their locks. 
I'm not clear
who will need it now,  since it doesn't affect the original read_lock() 
and write_lock()
I tried it.

Regards

Re: [patch] Add a writer prior lock methord for rwlock

[Liu Tao]

Oliver Neukum wrote:

>Am Mittwoch, 4. August 2004 11:09 schrieb Liu Tao:
>  
>
>>The patch adds the write_forcelock() methord, which has higher priority than
>>read_lock() and write_lock(). The original read_lock() and write_lock() 
>>is not
>>touched, and the unlock methord is still write_unlock();
>>    
>>
>
>It seems to me that with this a recursive read_lock() with
>a read lock already held may deadlock.
>
>	Regards
>
>		Oliver
>

Can you give an example path?
Thanks

Re: [patch] Add a writer prior lock methord for rwlock

[William Lee Irwin III]

On Wed, Aug 04, 2004 at 06:29:21PM +0800, Liu Tao wrote:
> write_forcelock() should be used to avoid readers starve writers, or for 
> writers
> to update shared data as far as possiable, since it prevents new readers 
> acquire
> the lock while it's waiting for existing readers release their locks. 
> I'm not clear
> who will need it now,  since it doesn't affect the original read_lock() 
> and write_lock()
> I tried it.

Unfortunately this will deadlock. read locks are acquired recursively.
The algorithms dependent on this need to be redesigned.


-- wli

Re: [patch] Add a writer prior lock methord for rwlock

[Liu Tao]

William Lee Irwin III wrote:

>Unfortunately this will deadlock. read locks are acquired recursively.
>The algorithms dependent on this need to be redesigned.
>
>  
>

Thanks, it's not a feasible scheme since read locks are recursive.

Regards

Re: [patch] Add a writer prior lock methord for rwlock

[Rik van Riel]

On Wed, 4 Aug 2004, Liu Tao wrote:

> write_forcelock() should be used to avoid readers starve writers, or for
> writers to update shared data as far as possiable, since it prevents new
> readers acquire the lock while it's waiting for existing readers release
> their locks.

Looks like it should deadlock if you have multiple CPUs
trying to upgrade from a read lock to a write lock
simultaneously...

-- 
"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it." - Brian W. Kernighan