[Cbe-oss-dev] Context switch question

[Arnd Bergmann]

On Tuesday 11 September 2007, Luke Browning wrote:

>         /* get syscall block from local store */
>         npc = ctx->ops->npc_read(ctx) & ~3;
>         ls = (void __iomem *)ctx->ops->get_ls(ctx);
>         ls_pointer = in_be32(ls + npc);
>         if (ls_pointer > (LS_SIZE - sizeof(s)))
>                 return -EFAULT;
>         memcpy_fromio(&s, ls + ls_pointer, sizeof(s));
> 
> // is this a potential deadlock during context switch since 
> // problem state mappings are removed and context lock is 
> // held by caller?  We would fault holding the lock and
> // the lock needs to be taken to resolve the fault. 

Thanks for sharing the observation, but I think this code
is perfectly correct as it is.
We don't access the problem state mappings here, so there also
is no way we could enter the page fault code at this point.

The get_ls() function returns a pointer to the _kernel_ mapping
of the local store, which is either pointing to the physical
local store, or to the lscsa. We need to hold the lock here to
be sure that mapping doesn't change under us.
 
> 
>         /* write result, jump over indirect pointer */
>         memcpy_toio(ls + ls_pointer, &spu_ret, sizeof(spu_ret));
> // same here?

yes.

> 
>         ctx->ops->npc_write(ctx, npc);
>         ctx->ops->runcntl_write(ctx, SPU_RUNCNTL_RUNNABLE);
> 
> // I think the npc_write and runcntl_write are OK, because the lock is held
> // meaning we are not in spu_save() or spu_restore().  However, we may be 
> // modifying the saved state of a context. 

Yes, that is intentional. The system call path itself does not depend on
the state being either saved or runnable, and neither does this code.

	Arnd <><