Messages in this thread Patch in this message | | | Date | Fri, 2 Mar 2012 22:20:37 +0800 | Subject | [PATCH] <down,down_interruptible...>, kernel <3.2.9> | From | Dennis Chen <> |
| |
Current down family functions use mismatch spin_lock pairs, this will incur some interrupt state chaos, for example, down_interruptible -- spin_lock_irqsave(&sem->lock, flags); P1 __down_common-- spin_unlock_irq(&sem->lock); P2 timeout = schedule_timeout(timeout); spin_lock_irq(&sem->lock); P3
spin_unlock_irqrestore(&sem->lock, flags); P4
Suppose 2 kernel thread A and B in an UP system call down_interruptible to get the semaphore, if the irq is _OFF_ before A calls, in the section between P2 and P3, the irq will be turned _ON_, then B begins to call down_interruptible, it will save a flag indicating irq is _ON_. So after A finish the path of down_interruptible, the irq is still _OFF_, but when B wakes up and finish the path, the irq will be _ON_. Actually, irq should be in on state before any down_interruptible calling, so spin_lock_irqsave/irqrestore is not necessary. Given it will make confusion for the reason of unmatched spin_lock pairs between down_interruptible and __down_common, so it's reason for the patch. Any comments?
--- semaphore.bak.c 2012-03-02 21:37:50.510302064 +0800 +++ semaphore.c 2012-03-02 21:46:32.330269122 +0800 @@ -54,12 +54,12 @@ { unsigned long flags;
- raw_spin_lock_irqsave(&sem->lock, flags); + raw_spin_lock_irq(&sem->lock, flags); if (likely(sem->count > 0)) sem->count--; else __down(sem); - raw_spin_unlock_irqrestore(&sem->lock, flags); + raw_spin_unlock_irq(&sem->lock, flags); } EXPORT_SYMBOL(down);
@@ -77,12 +77,12 @@ unsigned long flags; int result = 0;
- raw_spin_lock_irqsave(&sem->lock, flags); + raw_spin_lock_irq(&sem->lock, flags); if (likely(sem->count > 0)) sem->count--; else result = __down_interruptible(sem); - raw_spin_unlock_irqrestore(&sem->lock, flags); + raw_spin_unlock_irq(&sem->lock, flags);
return result; } @@ -103,12 +103,12 @@ unsigned long flags; int result = 0;
- raw_spin_lock_irqsave(&sem->lock, flags); + raw_spin_lock_irq(&sem->lock, flags); if (likely(sem->count > 0)) sem->count--; else result = __down_killable(sem); - raw_spin_unlock_irqrestore(&sem->lock, flags); + raw_spin_unlock_irq(&sem->lock, flags);
return result; } @@ -157,12 +157,12 @@ unsigned long flags; int result = 0;
- raw_spin_lock_irqsave(&sem->lock, flags); + raw_spin_lock_irq(&sem->lock, flags); if (likely(sem->count > 0)) sem->count--; else result = __down_timeout(sem, jiffies); - raw_spin_unlock_irqrestore(&sem->lock, flags); + raw_spin_unlock_irq(&sem->lock, flags);
return result; }
| |