On Mon, 28 Jun 2010 18:45:25 +0200
Manfred Spraul <manfred@xxxxxxxxxxxxxxxx> wrote:
> 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.
>
Thanks. BTW, cpu_relax() always includes asm("":::"memory") for avoiding
optimization ?
Thanks,
-Kame
--
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>