[Cbe-oss-dev] [PATCH 1/2] [POWERPC] spufs: fix context destruction during psmap fault

[Luke Browning]

On Thu, 2008-02-28 at 10:51 +1100, Jeremy Kerr wrote:
> We have a small window where a spu context may be destroyed while
> we're servicing a page fault (from another thread) to the context's
> problem state mapping.
> 
> After we up_read() the mmap_sem, it's possible that the context is
> destroyed by its owning thread, and so the later references to ctx
> are invalid. This can maifest as a deadlock on the (now free()-ed)
> context state mutex.
> 
> This change adds a reference to the context before we release the
> mmap_sem, so that the context cannot be destroyed.

I don't understand this fix.  The context pointer is loaded from an
inode.  Isn't this pointer supposed to be protected for the life of the
file?  We have a lot of interfaces that do this.   

What is the relationship with mmap_sem?  Your comment indicates that the
spu context structure can be destroyed because the code releases the
mmap_sem lock.  Why is that?  Is this lock taken during spu creation and
held over the life of the context?     

struct spu_context * get_spu_context(struct spu_context *ctx)
{
        kref_get(&ctx->kref);
        return ctx;
}

int put_spu_context(struct spu_context *ctx)
{
        return kref_put(&ctx->kref, &destroy_spu_context);
}

>From these routines, we see that the destroy routine is invoked automatically
when the last kref reference is released.  put_spu_context() is invoked from 
several places, including spufs_delete_inode(), affinity code in 
spufs_create_context(), and failure cases with spufs_create_context().

Andre, why are you invoking put_spu_context() in the affinity case?

I see that get_spu_context() is invoked for each file below the context structure, 
but it is not invoked for the parent or the file that represents the spu_context 
structure.  Is that a problem?  

Luke