2010-06-28 16:48:38

by Pekka Enberg

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

On Sat, Jun 26, 2010 at 12:20 AM, Christoph Lameter
<[email protected]> wrote:
> [Necessary to make 2.6.35-rc3 not deadlock. Not sure if this is the "right"(tm)
> fix]

Is this related to the SLUB patches? Regardless, lets add Andrew and
linux-kernel on CC.

> 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 <[email protected]>
> Signed-off-by: Christoph Lameter <[email protected]>
>
> ---
> ?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;
> +}
> +
> +
> ?SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
> ? ? ? ? ? ? ? ?unsigned, nsops, const struct timespec __user *, timeout)
> ?{
> @@ -1409,11 +1435,7 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
> ? ? ? ?else
> ? ? ? ? ? ? ? ?schedule();
>
> - ? ? ? error = queue.status;
> - ? ? ? while(unlikely(error == IN_WAKEUP)) {
> - ? ? ? ? ? ? ? cpu_relax();
> - ? ? ? ? ? ? ? error = queue.status;
> - ? ? ? }
> + ? ? ? error = get_queue_result(&queue);
>
> ? ? ? ?if (error != -EINTR) {
> ? ? ? ? ? ? ? ?/* fast path: update_queue already obtained all requested
> @@ -1427,10 +1449,12 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
> ? ? ? ? ? ? ? ?goto out_free;
> ? ? ? ?}
>
> + ? ? ? error = get_queue_result(&queue);
> +
> ? ? ? ?/*
> ? ? ? ? * If queue.status != -EINTR we are woken up by another process
> ? ? ? ? */
> - ? ? ? error = queue.status;
> +
> ? ? ? ?if (error != -EINTR) {
> ? ? ? ? ? ? ? ?goto out_unlock_free;
> ? ? ? ?}
> --
> 1.7.0.1
>
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. ?For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>


2010-06-29 16:23:48

by Christoph Lameter

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

This is a patch from Manfred. Required to make 2.6.35-rc3 work.