Re: [PATCH] fix race in mark_mounts_for_expiry()

[Ram <linuxram@us.ibm.com>]

On Wed, 2005-05-18 at 12:19, Miklos Szeredi wrote:
> > First of all the reason this race exists implies Al Viro may have had
> > assertion in his mind that "All tasks that have access to a namespace
> > has a refcount on that namespace".  If that was what he was thinking,
> > than the I would stick with that assertion being true. The overall idea
> > I am thinking off is:
> > 
> > 1) have the automounter hold a refcount on any new namespace created
> >     the mounts in which the automounter has interest in.
> > 2) have a refcount on the namespace when a new task gains access to
> >    a namespace through the file descriptor or any other 
> >    similar mechanisms and remove the reference
> >    once the fd gets closed. (bit tricky to implement)
> > 
> > Do you agree with the overall idea? 
> 
> I don't really understand it.
> 
> A reference count usually means, the number of references (pointers)
> to an object.  You can sometimes get away with schemes that deviate
> from this in various ways, but it's usually asking for trouble.

Ok. One more attempt to be clear.

All the places where get_namespace() is called currently except
in mark_mounts_for_expiry() are safe because they are called in
places where it is guaranteed that they will not race with 
__put_namespace(). 

For example in clone_namespace(), get_namespace() will not race
because the task that called the clone has a refcount on the
namespace and since that task is currently  in the kernel, there is
no chance for  the task  to go away  decrementing the refcount 
on that namespace. 

But the case where the call to get_namespace() is buggy in 
mark_mounts_for_expiry() is because:
it is called in a timer context, and the last process referring
the namespace may just disapper right than.  
 So what I am proposing is:
in automouter, while the automount takes place in 
afs_mntpt_follow_link() increment the refcount of the namespace,
by calling get_namespace(). This call will not race with __put_namespace
because the process that is trying to access the
mountpoint already has a refcount on the namespace and it won't be 
able to decrement the refcount currently. agree?

Now later when the automounter tries to unmount the mount 
call put_namespace() after unmounting. I mean do it in
mark_mounts_for_expiry(). Also delete the call to get_namespace()) 

So the race will not happen at all.

Makes sense? 

That was the easiest part, but the difficult part is how to call
get_namespace() on a forign namespace in do_loopback()?

thinking about it,
RP



> 
> The usage in mark_mounts_for_expiry() deviated from it so much, that
> the result was a subtle race.
> 
> Doing some tricky thing like what you propose will just likely
> introduce more subtle problems.
> 
> Miklos
> -
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/