[PATCH 3/5] powerpc/spufs: take a reference on contexts pulled off the runqueue

Junrui Luo via B4 Relay <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.feeds.b4-sent,org.kernel.vger.linux-kernel,org.ozlabs.lists.linuxppc-dev
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.