Revert "semaphore: fix"

This reverts commit bf726eab3711cf192405d21688a4b21e07b6188a, as it has been reported to cause a regression with processes stuck in __down(), apparently because some missing wakeup. Quoth Sven Wegener: "I'm currently investigating a regression that has showed up with my last git pull yesterday. Bisecting the commits showed bf726e "semaphore: fix" to be the culprit, reverting it fixed the issue. Symptoms: During heavy filesystem usage (e.g. a kernel compile) I get several compiler processes in uninterruptible sleep, blocking all i/o on the filesystem. System is an Intel Core 2 Quad running a 64bit kernel and userspace. Filesystem is xfs on top of lvm. See below for the output of sysrq-w." See http://lkml.org/lkml/2008/5/10/45 for full report. In the meantime, we can just fix the BKL performance regression by reverting back to the good old BKL spinlock implementation instead, since any sleeping lock will generally perform badly, especially if it tries to be fair. Reported-by: Sven Wegener <sven.wegener@stealer.net> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Ingo Molnar <mingo@elte.hu> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Revert "semaphore: fix"
This reverts commit bf726eab3711cf192405d21688a4b21e07b6188a, as it has been reported to cause a regression with processes stuck in __down(), apparently because some missing wakeup. Quoth Sven Wegener: "I'm currently investigating a regression that has showed up with my last git pull yesterday. Bisecting the commits showed bf726e "semaphore: fix" to be the culprit, reverting it fixed the issue. Symptoms: During heavy filesystem usage (e.g. a kernel compile) I get several compiler processes in uninterruptible sleep, blocking all i/o on the filesystem. System is an Intel Core 2 Quad running a 64bit kernel and userspace. Filesystem is xfs on top of lvm. See below for the output of sysrq-w." See http://lkml.org/lkml/2008/5/10/45 for full report. In the meantime, we can just fix the BKL performance regression by reverting back to the good old BKL spinlock implementation instead, since any sleeping lock will generally perform badly, especially if it tries to be fair. Reported-by: Sven Wegener <sven.wegener@stealer.net> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Ingo Molnar <mingo@elte.hu> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Linus Torvalds
1 parent 39f004ba27
Showing 1 changed file with 34 additions and 30 deletions Side-by-side Diff
kernel/semaphore.c
@@ -54,9 +54,10 @@
 	unsigned long flags;
  
 	spin_lock_irqsave(&sem->lock, flags);
-	if (unlikely(!sem->count))
+	if (likely(sem->count > 0))
+		sem->count--;
+	else
 		__down(sem);
-	sem->count--;
 	spin_unlock_irqrestore(&sem->lock, flags);
 }
 EXPORT_SYMBOL(down);
  
@@ -76,10 +77,10 @@
 	int result = 0;
  
 	spin_lock_irqsave(&sem->lock, flags);
-	if (unlikely(!sem->count))
-		result = __down_interruptible(sem);
-	if (!result)
+	if (likely(sem->count > 0))
 		sem->count--;
+	else
+		result = __down_interruptible(sem);
 	spin_unlock_irqrestore(&sem->lock, flags);
  
 	return result;
  
@@ -102,10 +103,10 @@
 	int result = 0;
  
 	spin_lock_irqsave(&sem->lock, flags);
-	if (unlikely(!sem->count))
-		result = __down_killable(sem);
-	if (!result)
+	if (likely(sem->count > 0))
 		sem->count--;
+	else
+		result = __down_killable(sem);
 	spin_unlock_irqrestore(&sem->lock, flags);
  
 	return result;
  
@@ -156,10 +157,10 @@
 	int result = 0;
  
 	spin_lock_irqsave(&sem->lock, flags);
-	if (unlikely(!sem->count))
-		result = __down_timeout(sem, jiffies);
-	if (!result)
+	if (likely(sem->count > 0))
 		sem->count--;
+	else
+		result = __down_timeout(sem, jiffies);
 	spin_unlock_irqrestore(&sem->lock, flags);
  
 	return result;
@@ -178,8 +179,9 @@
 	unsigned long flags;
  
 	spin_lock_irqsave(&sem->lock, flags);
-	sem->count++;
-	if (unlikely(!list_empty(&sem->wait_list)))
+	if (likely(list_empty(&sem->wait_list)))
+		sem->count++;
+	else
 		__up(sem);
 	spin_unlock_irqrestore(&sem->lock, flags);
 }
@@ -190,6 +192,7 @@
 struct semaphore_waiter {
 	struct list_head list;
 	struct task_struct *task;
+	int up;
 };
  
 /*
  
  
  
  
  
  
@@ -202,34 +205,33 @@
 {
 	struct task_struct *task = current;
 	struct semaphore_waiter waiter;
-	int ret = 0;
  
-	waiter.task = task;
 	list_add_tail(&waiter.list, &sem->wait_list);
+	waiter.task = task;
+	waiter.up = 0;
  
 	for (;;) {
-		if (state == TASK_INTERRUPTIBLE && signal_pending(task)) {
-			ret = -EINTR;
-			break;
-		}
-		if (state == TASK_KILLABLE && fatal_signal_pending(task)) {
-			ret = -EINTR;
-			break;
-		}
-		if (timeout <= 0) {
-			ret = -ETIME;
-			break;
-		}
+		if (state == TASK_INTERRUPTIBLE && signal_pending(task))
+			goto interrupted;
+		if (state == TASK_KILLABLE && fatal_signal_pending(task))
+			goto interrupted;
+		if (timeout <= 0)
+			goto timed_out;
 		__set_task_state(task, state);
 		spin_unlock_irq(&sem->lock);
 		timeout = schedule_timeout(timeout);
 		spin_lock_irq(&sem->lock);
-		if (sem->count > 0)
-			break;
+		if (waiter.up)
+			return 0;
 	}
  
+ timed_out:
 	list_del(&waiter.list);
-	return ret;
+	return -ETIME;
+
+ interrupted:
+	list_del(&waiter.list);
+	return -EINTR;
 }
  
 static noinline void __sched __down(struct semaphore *sem)
@@ -256,6 +258,8 @@
 {
 	struct semaphore_waiter *waiter = list_first_entry(&sem->wait_list,
 						struct semaphore_waiter, list);
+	list_del(&waiter->list);
+	waiter->up = 1;
 	wake_up_process(waiter->task);
 }
...	...	@@ -54,9 +54,10 @@
54	54	unsigned long flags;
55	55
56	56	spin_lock_irqsave(&sem->lock, flags);
57		- if (unlikely(!sem->count))
	57	+ if (likely(sem->count > 0))
	58	+ sem->count--;
	59	+ else
58	60	__down(sem);
59		- sem->count--;
60	61	spin_unlock_irqrestore(&sem->lock, flags);
61	62	}
62	63	EXPORT_SYMBOL(down);
63	64
...	...	@@ -76,10 +77,10 @@
76	77	int result = 0;
77	78
78	79	spin_lock_irqsave(&sem->lock, flags);
79		- if (unlikely(!sem->count))
80		- result = __down_interruptible(sem);
81		- if (!result)
	80	+ if (likely(sem->count > 0))
82	81	sem->count--;
	82	+ else
	83	+ result = __down_interruptible(sem);
83	84	spin_unlock_irqrestore(&sem->lock, flags);
84	85
85	86	return result;
86	87
...	...	@@ -102,10 +103,10 @@
102	103	int result = 0;
103	104
104	105	spin_lock_irqsave(&sem->lock, flags);
105		- if (unlikely(!sem->count))
106		- result = __down_killable(sem);
107		- if (!result)
	106	+ if (likely(sem->count > 0))
108	107	sem->count--;
	108	+ else
	109	+ result = __down_killable(sem);
109	110	spin_unlock_irqrestore(&sem->lock, flags);
110	111
111	112	return result;
112	113
...	...	@@ -156,10 +157,10 @@
156	157	int result = 0;
157	158
158	159	spin_lock_irqsave(&sem->lock, flags);
159		- if (unlikely(!sem->count))
160		- result = __down_timeout(sem, jiffies);
161		- if (!result)
	160	+ if (likely(sem->count > 0))
162	161	sem->count--;
	162	+ else
	163	+ result = __down_timeout(sem, jiffies);
163	164	spin_unlock_irqrestore(&sem->lock, flags);
164	165
165	166	return result;
...	...	@@ -178,8 +179,9 @@
178	179	unsigned long flags;
179	180
180	181	spin_lock_irqsave(&sem->lock, flags);
181		- sem->count++;
182		- if (unlikely(!list_empty(&sem->wait_list)))
	182	+ if (likely(list_empty(&sem->wait_list)))
	183	+ sem->count++;
	184	+ else
183	185	__up(sem);
184	186	spin_unlock_irqrestore(&sem->lock, flags);
185	187	}
...	...	@@ -190,6 +192,7 @@
190	192	struct semaphore_waiter {
191	193	struct list_head list;
192	194	struct task_struct *task;
	195	+ int up;
193	196	};
194	197
195	198	/*
196	199
197	200
198	201
199	202
200	203
201	204
...	...	@@ -202,34 +205,33 @@
202	205	{
203	206	struct task_struct *task = current;
204	207	struct semaphore_waiter waiter;
205		- int ret = 0;
206	208
207		- waiter.task = task;
208	209	list_add_tail(&waiter.list, &sem->wait_list);
	210	+ waiter.task = task;
	211	+ waiter.up = 0;
209	212
210	213	for (;;) {
211		- if (state == TASK_INTERRUPTIBLE && signal_pending(task)) {
212		- ret = -EINTR;
213		- break;
214		- }
215		- if (state == TASK_KILLABLE && fatal_signal_pending(task)) {
216		- ret = -EINTR;
217		- break;
218		- }
219		- if (timeout <= 0) {
220		- ret = -ETIME;
221		- break;
222		- }
	214	+ if (state == TASK_INTERRUPTIBLE && signal_pending(task))
	215	+ goto interrupted;
	216	+ if (state == TASK_KILLABLE && fatal_signal_pending(task))
	217	+ goto interrupted;
	218	+ if (timeout <= 0)
	219	+ goto timed_out;
223	220	__set_task_state(task, state);
224	221	spin_unlock_irq(&sem->lock);
225	222	timeout = schedule_timeout(timeout);
226	223	spin_lock_irq(&sem->lock);
227		- if (sem->count > 0)
228		- break;
	224	+ if (waiter.up)
	225	+ return 0;
229	226	}
230	227
	228	+ timed_out:
231	229	list_del(&waiter.list);
232		- return ret;
	230	+ return -ETIME;
	231	+
	232	+ interrupted:
	233	+ list_del(&waiter.list);
	234	+ return -EINTR;
233	235	}
234	236
235	237	static noinline void __sched __down(struct semaphore *sem)
...	...	@@ -256,6 +258,8 @@
256	258	{
257	259	struct semaphore_waiter *waiter = list_first_entry(&sem->wait_list,
258	260	struct semaphore_waiter, list);
	261	+ list_del(&waiter->list);
	262	+ waiter->up = 1;
259	263	wake_up_process(waiter->task);
260	264	}