Re: [patch 3/6] vfs: mountinfo stable peer group id

[Miklos Szeredi <miklos@szeredi.hu>]

> > > > From: Miklos Szeredi <mszeredi@suse.cz>
> > > Um?  Do you ever need to take it outside of vfsmount_lock?
> > > 
> > 
> > Tried to think this through:
> > 
> > It's always called with namespace_sem, which is enough, no need for a
> > new lock.  The bigger problem, is that it _is_ called with
> > vfsmount_lock in one case, which is bad, since the allocation may
> > sleep.
> 
> It is called with vfsmount_lock in *all* cases.  You've missed one
> in umount_tree(), BTW; you won't block in that case, though.

set_mnt_shared() is called from namespace.c as well, without
vfsmount_lock.  But agreed, that's not the real issue.

> 
> > That is in do_change_type().  But do we really need to hold
> > vfsmount_lock in that case?
> 
> Not the issue.
> 
> >  I think not, the propagation tree has no
> > relevance outside namespace_sem, so that one should be sufficient.
> 
> Callers manipulate more than propagation tree.  Note that e.g.
> umount_tree() changes all sorts of data structures, including ones
> that are traversed without namespace_sem.
> 
> I _really_ don't like the idea of different locking rules for caller
> of a function depending on the value of argument of that function.
> They are complicated enough as it is.
> 
> Argh...  OK, I'll try to put something together tonight, after I get some
> sleep - 31 hours of uptime _suck_ ;-/

Gosh, yes.

Thanks,
Miklos