Re: [PATCH] spufs raises two exceptions

[Arnd Bergmann <arnd@arndb.de>]

On Wednesday 07 March 2012, Al Viro wrote:
> > No it's not, it all goes together. spufs_create_context() always
> > unlocked & dropped the dentry before returning, so I assumed the
> > lock had to be dropped before fsnotify.
> > 
> > Note that if the problem is that the lock has to be dropped before
> > spu_forget(), then we should indeed move it back into the leaf functions
> > and just remove all the unlock path from the top ones. It's a bit nasty
> > how we drop the mutex first, then do spu_forget, then drop the dentry
> > but we could go back to doing that.
> > 
> > What I want is consistent semantics. It's just silly to have 3 different
> > stacking levels which all 3 may or may not be responsible to dropping
> > the lock & dentry depending on circumstances.
> 
> Why not leave unlock/dput to the caller?  Details of deadlocks caused
> by that approach, please...

If only I could remember that part exactly. I think I was remembering
0309f02d8 "spufs: fix deadlock in spu_create error path", which had a
double lock problem in this path. Please bear with me here because it's
been six years since I debugged that particular problem.

A lot of the nastyness of spu_forget() is about the problem of having
to give up a reference on current->mm that is held by an spu context,
while the mm contains a vma that maps this context and holds a refence
on the inode, in particular in case of calling exit().

However, I don't see now how either of these two issues requires that
we call spu_forget without the i_mutex held during a context creation
failure.

	Arnd