From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail203.messagelabs.com (mail203.messagelabs.com [216.82.254.243]) by kanga.kvack.org (Postfix) with SMTP id A4BE9600227 for ; Mon, 28 Jun 2010 12:44:45 -0400 (EDT) Received: by fxm17 with SMTP id 17so1461749fxm.14 for ; Mon, 28 Jun 2010 09:44:43 -0700 (PDT) Message-ID: <4C28D1A5.9040700@colorfullife.com> Date: Mon, 28 Jun 2010 18:45:25 +0200 From: Manfred Spraul MIME-Version: 1.0 Subject: Re: [S+Q 01/16] [PATCH] ipc/sem.c: Bugfix for semop() not reporting successful operation References: <20100625212026.810557229@quilx.com> <20100625212101.622422748@quilx.com> <20100628111731.18f1f858.kamezawa.hiroyu@jp.fujitsu.com> In-Reply-To: <20100628111731.18f1f858.kamezawa.hiroyu@jp.fujitsu.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org To: KAMEZAWA Hiroyuki Cc: Christoph Lameter , Pekka Enberg , linux-mm@kvack.org, Nick Piggin , Matt Mackall List-ID: On 06/28/2010 04:17 AM, KAMEZAWA Hiroyuki wrote: > On Fri, 25 Jun 2010 16:20:27 -0500 > Christoph Lameter 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 >> Signed-off-by: Christoph Lameter >> > > 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@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org