[Cbe-oss-dev] PATCH [4/7] decouple spu scheduler from spufs_spu_run [asynchronous scheduling]

[Luke Browning]

On Fri, 2007-11-23 at 14:13 +0100, Christoph Hellwig wrote:

> This deadlocks instantly once we overcommit spu ressources because
> spusched_tick runs with the list_mutex held and spu_unschedule/
> spu_schedule try to take it again.  
> 
> spu_unschedule could easily be fixed by adding a __spu_unschedule that
> expects the list_lock to be held, but for the spu_schedule case the
> code is more complex and we need to duplicate it in spusched_tick,
> so I bit the bullet and put an opencoded version of unbind+bind
> into spusched_tick where we can avoid decrementing nr_active just
> to increment it again and fuzz with the alloc_state.
> 
> With this fix most of Jeremy's testsuite passed except for some dma
> alignment tests.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>
> 
> 
> Index: linux-2.6/arch/powerpc/platforms/cell/spufs/sched.c
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/platforms/cell/spufs/sched.c	2007-11-23 12:37:05.000000000 +0100
> +++ linux-2.6/arch/powerpc/platforms/cell/spufs/sched.c	2007-11-23 13:06:12.000000000 +0100
> @@ -864,14 +869,21 @@
> 
>  		new = grab_runnable_context(ctx->prio + 1, spu->node);
>  		if (new) {
> -			spu_unschedule(spu, ctx);
> +			spu_unbind_context(spu, ctx);
> +			ctx->stats.invol_ctx_switch++;
> +			spu->stats.invol_ctx_switch++;
>  			spu_add_to_rq(ctx);
> +			mutex_unlock(&ctx->state_mutex);
> +
> +			mutex_lock(&new->state_mutex);
> +			spu_set_timeslice(new);
> +			spu_bind_context(spu, new);
> +			wake_up_all(&new->run_wq);
> +			mutex_unlock(&new->state_mutex);
>  		} else {
>  			spu_set_timeslice(ctx);
> +			mutex_unlock(&ctx->state_mutex);
>  		}
> -		mutex_unlock(&ctx->state_mutex);
> -		if (new)
> -			spu_schedule(spu, new);
>  	} else {
>  		ctx->time_slice++;
>  	}
> 

I think all we need to do is change the order of the patches to avoid
this deadlock.  "Patch [7/7] fix timeslicing" can be dropped ahead of
"Patch [4/7] decouple spu scheduler ...".  This provides a more elegant
solution IMO.

Regards,
Luke