From 9f43e3914dceb0f8191875b3cdf4325b48d0d70a Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Tue, 2 Sep 2008 11:57:09 +1000 Subject: powerpc/spufs: Fix multiple get_spu_context() Commit 8d5636fbca202f61fdb808fc9e20c0142291d802 introduced a reference count on SPU contexts during find_victim, but this may cause a leak in the reference count if we later find a better contender for a context to unschedule. Change the reference to after we've found our victim context, so we don't do the extra get_spu_context(). Signed-off-by: Jeremy Kerr --- arch/powerpc/platforms/cell/spufs/sched.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/powerpc/platforms/cell/spufs/sched.c b/arch/powerpc/platforms/cell/spufs/sched.c index 1c1b627ee843..9bb45c6b839c 100644 --- a/arch/powerpc/platforms/cell/spufs/sched.c +++ b/arch/powerpc/platforms/cell/spufs/sched.c @@ -643,9 +643,10 @@ static struct spu *find_victim(struct spu_context *ctx) !(tmp->flags & SPU_CREATE_NOSCHED) && (!victim || tmp->prio > victim->prio)) { victim = spu->ctx; - get_spu_context(victim); } } + if (victim) + get_spu_context(victim); mutex_unlock(&cbe_spu_info[node].list_mutex); if (victim) { -- cgit v1.2.2 From b65fe0356b5b732d7e1e0224c6a1cf2eb5255984 Mon Sep 17 00:00:00 2001 From: Jeremy Kerr Date: Thu, 4 Sep 2008 15:02:47 +1000 Subject: powerpc/spufs: Fix race for a free SPU We currently have a race for a free SPE. With one thread doing a spu_yield(), and another doing a spu_activate(): thread 1 thread 2 spu_yield(oldctx) spu_activate(ctx) __spu_deactivate(oldctx) spu_unschedule(oldctx, spu) spu->alloc_state = SPU_FREE spu = spu_get_idle(ctx) - searches for a SPE in state SPU_FREE, gets the context just freed by thread 1 spu_schedule(ctx, spu) spu->alloc_state = SPU_USED spu_schedule(newctx, spu) - assumes spu is still free - tries to schedule context on already-used spu This change introduces a 'free_spu' flag to spu_unschedule, to indicate whether or not the function should free the spu after descheduling the context. We only set this flag if we're not going to re-schedule another context on this SPU. Add a comment to document this behaviour. Signed-off-by: Jeremy Kerr --- arch/powerpc/platforms/cell/spufs/sched.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) (limited to 'arch') diff --git a/arch/powerpc/platforms/cell/spufs/sched.c b/arch/powerpc/platforms/cell/spufs/sched.c index 9bb45c6b839c..897c74061168 100644 --- a/arch/powerpc/platforms/cell/spufs/sched.c +++ b/arch/powerpc/platforms/cell/spufs/sched.c @@ -732,13 +732,28 @@ static void spu_schedule(struct spu *spu, struct spu_context *ctx) spu_release(ctx); } -static void spu_unschedule(struct spu *spu, struct spu_context *ctx) +/** + * spu_unschedule - remove a context from a spu, and possibly release it. + * @spu: The SPU to unschedule from + * @ctx: The context currently scheduled on the SPU + * @free_spu Whether to free the SPU for other contexts + * + * Unbinds the context @ctx from the SPU @spu. If @free_spu is non-zero, the + * SPU is made available for other contexts (ie, may be returned by + * spu_get_idle). If this is zero, the caller is expected to schedule another + * context to this spu. + * + * Should be called with ctx->state_mutex held. + */ +static void spu_unschedule(struct spu *spu, struct spu_context *ctx, + int free_spu) { int node = spu->node; mutex_lock(&cbe_spu_info[node].list_mutex); cbe_spu_info[node].nr_active--; - spu->alloc_state = SPU_FREE; + if (free_spu) + spu->alloc_state = SPU_FREE; spu_unbind_context(spu, ctx); ctx->stats.invol_ctx_switch++; spu->stats.invol_ctx_switch++; @@ -838,7 +853,7 @@ static int __spu_deactivate(struct spu_context *ctx, int force, int max_prio) if (spu) { new = grab_runnable_context(max_prio, spu->node); if (new || force) { - spu_unschedule(spu, ctx); + spu_unschedule(spu, ctx, new == NULL); if (new) { if (new->flags & SPU_CREATE_NOSCHED) wake_up(&new->stop_wq); @@ -911,7 +926,7 @@ static noinline void spusched_tick(struct spu_context *ctx) new = grab_runnable_context(ctx->prio + 1, spu->node); if (new) { - spu_unschedule(spu, ctx); + spu_unschedule(spu, ctx, 0); if (test_bit(SPU_SCHED_SPU_RUN, &ctx->sched_flags)) spu_add_to_rq(ctx); } else { -- cgit v1.2.2 From b2e601d14deb2083e2a537b47869ab3895d23a28 Mon Sep 17 00:00:00 2001 From: Andre Detsch Date: Thu, 4 Sep 2008 21:16:27 +0000 Subject: powerpc/spufs: Fix possible scheduling of a context to multiple SPEs We currently have a race when scheduling a context to a SPE - after we have found a runnable context in spusched_tick, the same context may have been scheduled by spu_activate(). This may result in a panic if we try to unschedule a context that has been freed in the meantime. This change exits spu_schedule() if the context has already been scheduled, so we don't end up scheduling it twice. Signed-off-by: Andre Detsch Signed-off-by: Jeremy Kerr --- arch/powerpc/platforms/cell/spufs/sched.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/powerpc/platforms/cell/spufs/sched.c b/arch/powerpc/platforms/cell/spufs/sched.c index 897c74061168..67595bc380dc 100644 --- a/arch/powerpc/platforms/cell/spufs/sched.c +++ b/arch/powerpc/platforms/cell/spufs/sched.c @@ -728,7 +728,8 @@ static void spu_schedule(struct spu *spu, struct spu_context *ctx) /* not a candidate for interruptible because it's called either from the scheduler thread or from spu_deactivate */ mutex_lock(&ctx->state_mutex); - __spu_schedule(spu, ctx); + if (ctx->state == SPU_STATE_SAVED) + __spu_schedule(spu, ctx); spu_release(ctx); } -- cgit v1.2.2 From 4ff23fa93011e2367fea056e72c92709178972d9 Mon Sep 17 00:00:00 2001 From: Hugh Dickins Date: Sun, 7 Sep 2008 00:35:48 +0100 Subject: powerpc: Fix rare boot build breakage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A make -j20 powerpc kernel build broke a couple of months ago saying: In file included from arch/powerpc/boot/gunzip_util.h:13, from arch/powerpc/boot/prpmc2800.c:21: arch/powerpc/boot/zlib.h:85: error: expected ‘:’, ‘,’, ‘;’, ‘}’ or ‘__attribute__’ before ‘*’ token arch/powerpc/boot/zlib.h:630: warning: type defaults to ‘int’ in declaration of ‘Byte’ arch/powerpc/boot/zlib.h:630: error: expected ‘;’, ‘,’ or ‘)’ before ‘*’ token It happened again yesterday: too rare for me to confirm the fix, but it looks like the list of dependants on gunzip_util.h was incomplete. Signed-off-by: Hugh Dickins Signed-off-by: Paul Mackerras --- arch/powerpc/boot/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile index 14174aa24074..717a3bc1352e 100644 --- a/arch/powerpc/boot/Makefile +++ b/arch/powerpc/boot/Makefile @@ -49,7 +49,7 @@ zlib := inffast.c inflate.c inftrees.c zlibheader := inffast.h inffixed.h inflate.h inftrees.h infutil.h zliblinuxheader := zlib.h zconf.h zutil.h -$(addprefix $(obj)/,$(zlib) gunzip_util.o main.o): \ +$(addprefix $(obj)/,$(zlib) cuboot-c2k.o gunzip_util.o main.o prpmc2800.o): \ $(addprefix $(obj)/,$(zliblinuxheader)) $(addprefix $(obj)/,$(zlibheader)) src-libfdt := fdt.c fdt_ro.c fdt_wip.c fdt_sw.c fdt_rw.c fdt_strerror.c -- cgit v1.2.2