[PATCH 3/5] powerpc/spufs: take a reference on contexts pulled off the runqueue
Junrui Luo via B4 Relay <[email protected]>
| Newsgroups | org.ozlabs.lists.linuxppc-dev,org.kernel.feeds.b4-sent,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
From: Junrui Luo <[email protected]> grab_runnable_context() unlinks the chosen context with __spu_del_from_rq() and returns it after dropping spu_prio->runq_lock. The runqueue holds no reference of its own — __spu_add_to_rq() only does a list_add_tail() and __spu_del_from_rq() only a list_del_init() — so the caller is left with a bare pointer. Both callers dereference it after the lock is gone: __spu_deactivate() and spusched_tick() call spu_schedule(), which starts with mutex_lock(&ctx->state_mutex). The only thing synchronizing the two sides is spu_run_fini() -> spu_del_from_rq(), which takes runq_lock. Once grab_runnable_context() has unlinked the context, that call finds ctx->rq already empty and becomes a no-op, so the owner is free to leave spu_run() and close the context directory; the resulting put_spu_context() can reach destroy_spu_context() and kfree() while the scheduler still holds the pointer. The window spans a full spu_unschedule() -> spu_unbind_context() SPU context save, and the BUG_ON(!list_empty(&ctx->rq)) in destroy_spu_context() cannot catch it because list_del_init() has already emptied ctx->rq. Fix by taking a reference in grab_runnable_context() while runq_lock is still held, where a queued context is guaranteed to be alive, and dropping it in both callers once they are done with it. This matches what find_victim() already does around its state_mutex trylock. Fixes: e65c2f6fcebb ("[POWERPC] spufs: decouple spu scheduler from spufs_spu_run (asynchronous scheduling)") Reported-by: Yuhao Jiang <[email protected]> Assisted-by: Claude:claude-opus-5 Cc: [email protected] Signed-off-by: Junrui Luo <[email protected]> --- Found by inspection; I have no Cell/PS3 hardware, so this is compile-tested only. --- arch/powerpc/platforms/cell/spufs/sched.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/cell/spufs/sched.c b/arch/powerpc/platforms/cell/spufs/sched.c index c52af883e01c..1b50b28cab11 100644 --- a/arch/powerpc/platforms/cell/spufs/sched.c +++ b/arch/powerpc/platforms/cell/spufs/sched.c @@ -815,6 +815,9 @@ int spu_activate(struct spu_context *ctx, unsigned long flags) * * Remove the highest priority context on the runqueue and return it * to the caller. Returns %NULL if no runnable context was found. + * + * The context is returned with a reference held on behalf of the caller, + * which has to drop it using put_spu_context() once it is done with it. */ static struct spu_context *grab_runnable_context(int prio, int node) { @@ -830,6 +833,7 @@ static struct spu_context *grab_runnable_context(int prio, int node) /* XXX(hch): check for affinity here as well */ if (__node_allowed(ctx, node)) { __spu_del_from_rq(ctx); + get_spu_context(ctx); goto found; } } @@ -860,6 +864,7 @@ static int __spu_deactivate(struct spu_context *ctx, int force, int max_prio) interruptible */ mutex_lock(&ctx->state_mutex); } + put_spu_context(new); } } } @@ -933,8 +938,10 @@ static noinline void spusched_tick(struct spu_context *ctx) out: spu_release(ctx); - if (new) + if (new) { spu_schedule(spu, new); + put_spu_context(new); + } } /** -- 2.51.2