[PATCH 1/2] Introduce write_trylock_irqsave

[PATCH 1/2] Introduce write_trylock_irqsave

[Sripathi Kodi]

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 <sripathik@in.ibm.com>
---
 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

Re: [PATCH 1/2] Introduce write_trylock_irqsave

[Satyam Sharma]

On 5/9/07, Sripathi Kodi <sripathik@in.ibm.com> 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 <ssatyam@cse.iitk.ac.in>

---

 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.