2007-05-09 08:42:27

by Sripathi Kodi

[permalink] [raw]
Subject: [PATCH 1/2] Introduce write_trylock_irqsave

Hi,

I am trying to fix the BUG I mentioned here:
http://lkml.org/lkml/2007/04/20/41. I noticed that an elegant way to solve
this problem is to have a write_trylock_irqsave helper function. Since we
don't have this now, the code in ptrace_attach implements it using
local_irq_disable and write_trylock. I wish to add write_trylock_irqsave to
mainline kernel and then fix the -rt specific problem using this.

The patch below adds write_trylock_irqsave function.

Signed-off-by: Sripathi Kodi <[email protected]>
---
include/linux/spinlock.h | 2 ++
include/linux/spinlock_api_smp.h | 1 +
include/linux/spinlock_api_up.h | 2 ++
kernel/spinlock.c | 14 ++++++++++++++
4 files changed, 19 insertions(+)

Index: linux-2.6.21/include/linux/spinlock.h
===================================================================
--- linux-2.6.21.orig/include/linux/spinlock.h
+++ linux-2.6.21/include/linux/spinlock.h
@@ -171,6 +171,8 @@ do { \
#define spin_trylock(lock) __cond_lock(lock, _spin_trylock(lock))
#define read_trylock(lock) __cond_lock(lock, _read_trylock(lock))
#define write_trylock(lock) __cond_lock(lock, _write_trylock(lock))
+#define write_trylock_irqsave(lock, flags) \
+ __cond_lock(lock, _write_trylock_irqsave(lock, flags))

#define spin_lock(lock) _spin_lock(lock)

Index: linux-2.6.21/include/linux/spinlock_api_smp.h
===================================================================
--- linux-2.6.21.orig/include/linux/spinlock_api_smp.h
+++ linux-2.6.21/include/linux/spinlock_api_smp.h
@@ -41,6 +41,7 @@ unsigned long __lockfunc _write_lock_irq
int __lockfunc _spin_trylock(spinlock_t *lock);
int __lockfunc _read_trylock(rwlock_t *lock);
int __lockfunc _write_trylock(rwlock_t *lock);
+int __lockfunc _write_trylock_irqsave(rwlock_t *lock, unsigned long flags);
int __lockfunc _spin_trylock_bh(spinlock_t *lock);
void __lockfunc _spin_unlock(spinlock_t *lock) __releases(lock);
void __lockfunc _read_unlock(rwlock_t *lock) __releases(lock);
Index: linux-2.6.21/include/linux/spinlock_api_up.h
===================================================================
--- linux-2.6.21.orig/include/linux/spinlock_api_up.h
+++ linux-2.6.21/include/linux/spinlock_api_up.h
@@ -64,6 +64,8 @@
#define _spin_trylock(lock) ({ __LOCK(lock); 1; })
#define _read_trylock(lock) ({ __LOCK(lock); 1; })
#define _write_trylock(lock) ({ __LOCK(lock); 1; })
+#define _write_trylock_irqsave(lock, flags) \
+ ({ __LOCK_IRQSAVE(lock, flags); 1; })
#define _spin_trylock_bh(lock) ({ __LOCK_BH(lock); 1; })
#define _spin_unlock(lock) __UNLOCK(lock)
#define _read_unlock(lock) __UNLOCK(lock)
Index: linux-2.6.21/kernel/spinlock.c
===================================================================
--- linux-2.6.21.orig/kernel/spinlock.c
+++ linux-2.6.21/kernel/spinlock.c
@@ -60,6 +60,20 @@ int __lockfunc _write_trylock(rwlock_t *
}
EXPORT_SYMBOL(_write_trylock);

+int __lockfunc _write_trylock_irqsave(rwlock_t *lock, unsigned long flags)
+{
+ int ret;
+
+ local_irq_save(flags);
+ ret = _write_trylock(lock);
+ if (ret)
+ return ret;
+
+ local_irq_restore(flags);
+ return 0;
+}
+EXPORT_SYMBOL(_write_trylock_irqsave);
+
/*
* If lockdep is enabled then we use the non-preemption spin-ops
* even on CONFIG_PREEMPT, because lockdep assumes that interrupts are


2007-05-10 03:18:44

by Satyam Sharma

[permalink] [raw]
Subject: Re: [PATCH 1/2] Introduce write_trylock_irqsave

On 5/9/07, Sripathi Kodi <[email protected]> wrote:
> Hi,
>
> I am trying to fix the BUG I mentioned here:
> http://lkml.org/lkml/2007/04/20/41. I noticed that an elegant way to solve
> this problem is to have a write_trylock_irqsave helper function. Since we
> don't have this now, the code in ptrace_attach implements it using
> local_irq_disable and write_trylock. I wish to add write_trylock_irqsave to
> mainline kernel and then fix the -rt specific problem using this.
>
> The patch below adds write_trylock_irqsave function.

Why not implement this as follows? That way we're able to (1) be
consistent in style with spin_trylock_irqsave, (2) type fewer lines,
(3) touch fewer files, and (4) not add extra bulk to kernel text size
either.

For the sake of completeness of the API, we could also add a
read_trylock_irqsave, but I'd be against the _irq variants (or should
we call them hazards) that don't save / restore.

Signed-off-by: Satyam Sharma <[email protected]>

---

include/linux/spinlock.h | 7 +++++++
1 file changed, 7 insertions(+)

---

diff -ruNp a/include/linux/spinlock.h b/include/linux/spinlock.h
--- a/include/linux/spinlock.h 2007-04-26 08:38:32.000000000 +0530
+++ b/include/linux/spinlock.h 2007-05-10 08:43:32.000000000 +0530
@@ -282,6 +282,13 @@ do { \
1 : ({ local_irq_restore(flags); 0; }); \
})

+#define write_trylock_irqsave(lock, flags) \
+({ \
+ local_irq_save(flags); \
+ write_trylock(lock) ? \
+ 1 : ({ local_irq_restore(flags); 0; }); \
+})
+
/*
* Locks two spinlocks l1 and l2.
* l1_first indicates if spinlock l1 should be taken first.