[Cbe-oss-dev] [PATCH] fix state_mutex leaks
Luke Browning
lukebr at linux.vnet.ibm.com
Thu Feb 7 04:49:25 EST 2008
On Wed, 2008-02-06 at 07:42 +0100, Christoph Hellwig wrote:
Signed-off-by: Luke Browning <lukebrowning at us.ibm.com>
>
> Index: powerpc/arch/powerpc/platforms/cell/spufs/fault.c
> ===================================================================
> --- powerpc.orig/arch/powerpc/platforms/cell/spufs/fault.c 2008-02-06 07:21:50.000000000 +0100
> +++ powerpc/arch/powerpc/platforms/cell/spufs/fault.c 2008-02-06 07:26:09.000000000 +0100
> @@ -108,7 +108,7 @@ int spufs_handle_class1(struct spu_conte
> u64 ea, dsisr, access;
> unsigned long flags;
> unsigned flt = 0;
> - int ret, ret2;
> + int ret;
>
> /*
> * dar and dsisr get passed from the registers
> @@ -148,13 +148,11 @@ int spufs_handle_class1(struct spu_conte
> ret = spu_handle_mm_fault(current->mm, ea, dsisr, &flt);
>
> /*
> - * If spu_acquire fails due to a pending signal we just want to return
> - * EINTR to userspace even if that means missing the dma restart or
> - * updating the page fault statistics.
> + * This is nasty: we need the state_mutex for all the
> + * bookkeeping even if the syscall was interrupted by
> + * a signal. ewww.
> */
> - ret2 = spu_acquire(ctx);
> - if (ret2)
> - goto out;
> + mutex_lock(&ctx->state_mutex);
>
> /*
> * Clear dsisr under ctxt lock after handling the fault, so that
> @@ -185,7 +183,6 @@ int spufs_handle_class1(struct spu_conte
> } else
> spufs_handle_event(ctx, ea, SPE_EVENT_SPE_DATA_STORAGE);
>
> - out:
> spuctx_switch_state(ctx, SPU_UTIL_SYSTEM);
> return ret;
> }
> Index: powerpc/arch/powerpc/platforms/cell/spufs/file.c
> ===================================================================
> --- powerpc.orig/arch/powerpc/platforms/cell/spufs/file.c 2008-02-06 07:21:50.000000000 +0100
> +++ powerpc/arch/powerpc/platforms/cell/spufs/file.c 2008-02-06 07:25:29.000000000 +0100
> @@ -357,6 +357,7 @@ static unsigned long spufs_ps_nopfn(stru
> {
> struct spu_context *ctx = vma->vm_file->private_data;
> unsigned long area, offset = address - vma->vm_start;
> + int ret = 0;
>
> offset += vma->vm_pgoff << PAGE_SHIFT;
> if (offset >= ps_size)
> @@ -375,14 +376,15 @@ static unsigned long spufs_ps_nopfn(stru
>
> if (ctx->state == SPU_STATE_SAVED) {
> up_read(¤t->mm->mmap_sem);
> - spufs_wait(ctx->run_wq, ctx->state == SPU_STATE_RUNNABLE);
> + ret = spufs_wait(ctx->run_wq, ctx->state == SPU_STATE_RUNNABLE);
> down_read(¤t->mm->mmap_sem);
> } else {
> area = ctx->spu->problem_phys + ps_offs;
> vm_insert_pfn(vma, address, (area + offset) >> PAGE_SHIFT);
> }
>
> - spu_release(ctx);
> + if (!ret)
> + spu_release(ctx);
> return NOPFN_REFAULT;
> }
>
> @@ -749,23 +751,25 @@ static ssize_t spufs_ibox_read(struct fi
>
> count = spu_acquire(ctx);
> if (count)
> - return count;
> + goto out;
>
> /* wait only for the first element */
> count = 0;
> if (file->f_flags & O_NONBLOCK) {
> - if (!spu_ibox_read(ctx, &ibox_data))
> + if (!spu_ibox_read(ctx, &ibox_data)) {
> count = -EAGAIN;
> + goto out_unlock;
> + }
> } else {
> count = spufs_wait(ctx->ibox_wq, spu_ibox_read(ctx, &ibox_data));
> + if (count)
> + goto out;
> }
> - if (count)
> - goto out;
>
> /* if we can't write at all, return -EFAULT */
> count = __put_user(ibox_data, udata);
> if (count)
> - goto out;
> + goto out_unlock;
>
> for (count = 4, udata++; (count + 4) <= len; count += 4, udata++) {
> int ret;
> @@ -782,9 +786,9 @@ static ssize_t spufs_ibox_read(struct fi
> break;
> }
>
> -out:
> +out_unlock:
> spu_release(ctx);
> -
> +out:
> return count;
> }
>
> @@ -899,7 +903,7 @@ static ssize_t spufs_wbox_write(struct f
>
> count = spu_acquire(ctx);
> if (count)
> - return count;
> + goto out;
>
> /*
> * make sure we can at least write one element, by waiting
> @@ -907,14 +911,16 @@ static ssize_t spufs_wbox_write(struct f
> */
> count = 0;
> if (file->f_flags & O_NONBLOCK) {
> - if (!spu_wbox_write(ctx, wbox_data))
> + if (!spu_wbox_write(ctx, wbox_data)) {
> count = -EAGAIN;
> + goto out_unlock;
> + }
> } else {
> count = spufs_wait(ctx->wbox_wq, spu_wbox_write(ctx, wbox_data));
> + if (count)
> + goto out;
> }
>
> - if (count)
> - goto out;
>
> /* write as much as possible */
> for (count = 4, udata++; (count + 4) <= len; count += 4, udata++) {
> @@ -928,8 +934,9 @@ static ssize_t spufs_wbox_write(struct f
> break;
> }
>
> -out:
> +out_unlock:
> spu_release(ctx);
> +out:
> return count;
> }
>
> @@ -1592,12 +1599,11 @@ static ssize_t spufs_mfc_read(struct fil
> } else {
> ret = spufs_wait(ctx->mfc_wq,
> spufs_read_mfc_tagstatus(ctx, &status));
> + if (ret)
> + goto out;
> }
> spu_release(ctx);
>
> - if (ret)
> - goto out;
> -
> ret = 4;
> if (copy_to_user(buffer, &status, 4))
> ret = -EFAULT;
> @@ -1726,6 +1732,8 @@ static ssize_t spufs_mfc_write(struct fi
> int status;
> ret = spufs_wait(ctx->mfc_wq,
> spu_send_mfc_command(ctx, cmd, &status));
> + if (ret)
> + goto out;
> if (status)
> ret = status;
> }
> @@ -1779,7 +1787,7 @@ static int spufs_mfc_flush(struct file *
>
> ret = spu_acquire(ctx);
> if (ret)
> - return ret;
> + goto out;
> #if 0
> /* this currently hangs */
> ret = spufs_wait(ctx->mfc_wq,
> @@ -1788,12 +1796,13 @@ static int spufs_mfc_flush(struct file *
> goto out;
> ret = spufs_wait(ctx->mfc_wq,
> ctx->ops->read_mfc_tagstatus(ctx) == ctx->tagwait);
> -out:
> + if (ret)
> + goto out;
> #else
> ret = 0;
> #endif
> spu_release(ctx);
> -
> +out:
> return ret;
> }
>
> Index: powerpc/arch/powerpc/platforms/cell/spufs/run.c
> ===================================================================
> --- powerpc.orig/arch/powerpc/platforms/cell/spufs/run.c 2008-02-06 07:21:50.000000000 +0100
> +++ powerpc/arch/powerpc/platforms/cell/spufs/run.c 2008-02-06 07:26:23.000000000 +0100
> @@ -354,8 +354,15 @@ long spufs_run_spu(struct spu_context *c
>
> do {
> ret = spufs_wait(ctx->stop_wq, spu_stopped(ctx, &status));
> - if (unlikely(ret))
> + if (unlikely(ret)) {
> + /*
> + * This is nasty: we need the state_mutex for all the
> + * bookkeeping even if the syscall was interrupted by
> + * a signal. ewww.
> + */
> + mutex_lock(&ctx->state_mutex);
> break;
> + }
> spu = ctx->spu;
> if (unlikely(test_and_clear_bit(SPU_SCHED_NOTIFY_ACTIVE,
> &ctx->sched_flags))) {
> Index: powerpc/arch/powerpc/platforms/cell/spufs/spufs.h
> ===================================================================
> --- powerpc.orig/arch/powerpc/platforms/cell/spufs/spufs.h 2008-02-06 07:21:50.000000000 +0100
> +++ powerpc/arch/powerpc/platforms/cell/spufs/spufs.h 2008-02-06 07:23:53.000000000 +0100
> @@ -268,6 +268,9 @@ extern char *isolated_loader;
> * Same as wait_event_interruptible(), except that here
> * we need to call spu_release(ctx) before sleeping, and
> * then spu_acquire(ctx) when awoken.
> + *
> + * Returns with state_mutex re-acquired when successfull or
> + * with -ERESTARTSYS and the state_mutex dropped when interrupted.
> */
>
> #define spufs_wait(wq, condition) \
> @@ -278,11 +281,11 @@ extern char *isolated_loader;
> prepare_to_wait(&(wq), &__wait, TASK_INTERRUPTIBLE); \
> if (condition) \
> break; \
> + spu_release(ctx); \
> if (signal_pending(current)) { \
> __ret = -ERESTARTSYS; \
> break; \
> } \
> - spu_release(ctx); \
> schedule(); \
> __ret = spu_acquire(ctx); \
> if (__ret) \
More information about the cbe-oss-dev
mailing list