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

[Pekka Enberg]

On Sat, Jun 26, 2010 at 12:20 AM, Christoph Lameter
<cl@xxxxxxxxxxxxxxxxxxxx> 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 <manfred@xxxxxxxxxxxxxxxx>
> Signed-off-by: Christoph Lameter <cl@xxxxxxxxxxxxxxxxxxxx>
>
> ---
>  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 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>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/