[PATCH] rwsem generic spinlock: use IRQ save/restore spinlocks

[PATCH] rwsem generic spinlock: use IRQ save/restore spinlocks

[Kevin Hilman]

rwsems can be used with IRQs disabled, particularily in early boot
before IRQs are enabled.  Currently the spin_unlock_irq() usage in the
slow-path will unconditionally enable interrupts and cause problems
early in boot where interrupts are not yet initialized or enabled.

This patch uses save/restore versions of IRQ spinlocks in the slowpath
to ensure interrupts are not unintentionally enabled in the case where
the rwsem is used with IRQs disabled.

Idea for this fix suggested by H. Peter Anvin.

Tested on TI OMAP3-based platform (ARM Cortex-A8)

Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Rabin Vincent <rabin@rab.in>
Cc: H. Peter Anvin <hpa@zytor.com>
LKML-Reference: <alpine.LFD.2.00.1004010904540.3707@i5.linux-foundation.org>
Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>
---
 lib/rwsem-spinlock.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/lib/rwsem-spinlock.c b/lib/rwsem-spinlock.c
index ccf95bf..ffc9fc7 100644
--- a/lib/rwsem-spinlock.c
+++ b/lib/rwsem-spinlock.c
@@ -143,13 +143,14 @@ void __sched __down_read(struct rw_semaphore *sem)
 {
 	struct rwsem_waiter waiter;
 	struct task_struct *tsk;
+	unsigned long flags;
 
-	spin_lock_irq(&sem->wait_lock);
+	spin_lock_irqsave(&sem->wait_lock, flags);
 
 	if (sem->activity >= 0 && list_empty(&sem->wait_list)) {
 		/* granted */
 		sem->activity++;
-		spin_unlock_irq(&sem->wait_lock);
+		spin_unlock_irqrestore(&sem->wait_lock, flags);
 		goto out;
 	}
 
@@ -164,7 +165,7 @@ void __sched __down_read(struct rw_semaphore *sem)
 	list_add_tail(&waiter.list, &sem->wait_list);
 
 	/* we don't need to touch the semaphore struct anymore */
-	spin_unlock_irq(&sem->wait_lock);
+	spin_unlock_irqrestore(&sem->wait_lock, flags);
 
 	/* wait to be given the lock */
 	for (;;) {
@@ -209,13 +210,14 @@ void __sched __down_write_nested(struct rw_semaphore *sem, int subclass)
 {
 	struct rwsem_waiter waiter;
 	struct task_struct *tsk;
+	unsigned long flags;
 
-	spin_lock_irq(&sem->wait_lock);
+	spin_lock_irqsave(&sem->wait_lock, flags);
 
 	if (sem->activity == 0 && list_empty(&sem->wait_list)) {
 		/* granted */
 		sem->activity = -1;
-		spin_unlock_irq(&sem->wait_lock);
+		spin_unlock_irqrestore(&sem->wait_lock, flags);
 		goto out;
 	}
 
@@ -230,7 +232,7 @@ void __sched __down_write_nested(struct rw_semaphore *sem, int subclass)
 	list_add_tail(&waiter.list, &sem->wait_list);
 
 	/* we don't need to touch the semaphore struct anymore */
-	spin_unlock_irq(&sem->wait_lock);
+	spin_unlock_irqrestore(&sem->wait_lock, flags);
 
 	/* wait to be given the lock */
 	for (;;) {
-- 
1.7.0.2

Re: [PATCH] rwsem generic spinlock: use IRQ save/restore spinlocks

[Kevin Hilman]

Kevin Hilman <khilman@deeprootsystems.com> writes:

> rwsems can be used with IRQs disabled, particularily in early boot
> before IRQs are enabled.  Currently the spin_unlock_irq() usage in the
> slow-path will unconditionally enable interrupts and cause problems
> early in boot where interrupts are not yet initialized or enabled.
>
> This patch uses save/restore versions of IRQ spinlocks in the slowpath
> to ensure interrupts are not unintentionally enabled in the case where
> the rwsem is used with IRQs disabled.
>
> Idea for this fix suggested by H. Peter Anvin.
>
> Tested on TI OMAP3-based platform (ARM Cortex-A8)
>
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
> Cc: Rabin Vincent <rabin@rab.in>
> Cc: H. Peter Anvin <hpa@zytor.com>
> LKML-Reference: <alpine.LFD.2.00.1004010904540.3707@i5.linux-foundation.org>
> Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>

For the benefit of the l-o archives...

This patch was merged into mainline in .34-rc6 and fixes potential
deadlocks on boot, seen especially when using the SLUB allocator.

Kevin