Re: [S+Q 01/16] [PATCH] ipc/sem.c: Bugfix for semop() not reporting successful operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/28/2010 04:17 AM, KAMEZAWA Hiroyuki wrote:
On Fri, 25 Jun 2010 16:20:27 -0500
Christoph Lameter<cl@xxxxxxxxxxxxxxxxxxxx>  wrote:

[Necessary to make 2.6.35-rc3 not deadlock. Not sure if this is the "right"(tm)
fix]

The last change to improve the scalability moved the actual wake-up out of
the section that is protected by spin_lock(sma->sem_perm.lock).

This means that IN_WAKEUP can be in queue.status even when the spinlock is
acquired by the current task. Thus the same loop that is performed when
queue.status is read without the spinlock acquired must be performed when
the spinlock is acquired.

Signed-off-by: Manfred Spraul<manfred@xxxxxxxxxxxxxxxx>
Signed-off-by: Christoph Lameter<cl@xxxxxxxxxxxxxxxxxxxx>

Hmm, I'm sorry if I don't understand the code...

---
  ipc/sem.c |   36 ++++++++++++++++++++++++++++++------
  1 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/ipc/sem.c b/ipc/sem.c
index 506c849..523665f 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1256,6 +1256,32 @@ out:
  	return un;
  }

+
+/** get_queue_result - Retrieve the result code from sem_queue
+ * @q: Pointer to queue structure
+ *
+ * The function retrieve the return code from the pending queue. If
+ * IN_WAKEUP is found in q->status, then we must loop until the value
+ * is replaced with the final value: This may happen if a task is
+ * woken up by an unrelated event (e.g. signal) and in parallel the task
+ * is woken up by another task because it got the requested semaphores.
+ *
+ * The function can be called with or without holding the semaphore spinlock.
+ */
+static int get_queue_result(struct sem_queue *q)
+{
+	int error;
+
+	error = q->status;
+	while(unlikely(error == IN_WAKEUP)) {
+		cpu_relax();
+		error = q->status;
+	}
+
+	return error;
+}
no memory barrier is required ?

No.
q->status is the only field that is read in the exit path of sys_semtimedop(): After that, q->status is used as the return value of sys_semtimedop(), without accessing any other field. Thus no memory barrier is required: there is just no other read/write operation against which the read of q->status must be serialized.

There is a smp_wmb() wake_up_sem_queue_do(), to ensure that all writes that are done by the cpu that does the wake-up are completed before q->status is set to the final value.

--
    Manfred

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>


[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]