[Cbe-oss-dev] [PATCH] fix state_mutex leaks
Christoph Hellwig
hch at lst.de
Fri Jan 18 00:51:22 EST 2008
Fix various state_mutex leaks. The worst one was introduced by the
interrutible state_mutex conversion but there've been a few before
aswell. Notably spufs_wait now returns without the state_mutex held
when returning an error, which actually cleans up some code aswell.
Signed-off-by: Christoph Hellwig <hch at lst.de>
Index: powerpc/arch/powerpc/platforms/cell/spufs/fault.c
===================================================================
--- powerpc.orig/arch/powerpc/platforms/cell/spufs/fault.c 2008-01-14 10:56:24.000000000 +0100
+++ powerpc/arch/powerpc/platforms/cell/spufs/fault.c 2008-01-14 18:12:26.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
+ * bookkeping 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-01-14 18:11:42.000000000 +0100
+++ powerpc/arch/powerpc/platforms/cell/spufs/file.c 2008-01-17 14:49:01.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;
}
@@ -1589,15 +1596,15 @@ static ssize_t spufs_mfc_read(struct fil
else
/* XXX(hch): shouldn't we clear ret here? */
ctx->tagwait &= ~status;
+ spu_release(ctx);
} 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 +1733,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 +1788,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 +1797,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-01-14 10:56:24.000000000 +0100
+++ powerpc/arch/powerpc/platforms/cell/spufs/run.c 2008-01-17 14:49:01.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
+ * bookkeping 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-01-14 18:11:42.000000000 +0100
+++ powerpc/arch/powerpc/platforms/cell/spufs/spufs.h 2008-01-17 14:49:01.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