On Thu, Jan 12, 2017 at 05:03:42AM +1300, Eric W. Biederman wrote:
> Al Viro <viro@xxxxxxxxxxxxxxxxxx> writes:
>
> > On Wed, Jan 11, 2017 at 01:10:57PM +1300, Eric W. Biederman wrote:
> >> >> + if (child->mnt.mnt_root == smp->m_dentry) {
> >> >
> >> > Explain, please. In which case is that condition _not_ satisfied, and
> >> > what should happen i
> >>
> >> When a tree is grafted in that condition does not apply to the lower
> >> leaves of the tree. At the same time nothing needs to be done for those
> >> leaves. Only the primary mountpoint needs to worry about tucking.
> >
> > How in hell would those lower leaves end up on the list in
> > attach_recursive_mnt()? IDGI...
>
> The submounts of a mount tree that is being attached need to have
> commit_tree called on them to attach them to a mount namespace.
Huh? commit_tree() is called once for each copy of the source tree. This
list_add_tail(&head, &mnt->mnt_list);
list_for_each_entry(m, &head, mnt_list)
m->mnt_ns = n;
is what goes through submounts in each of them, _not_ the loop in the caller.
What we get out of propagate_mnt() is the list of copies of source tree,
one for each of the mountpoints that should get propagation from the
target.
->mnt_mountpoint/->mnt_parent is fully set for all nodes.
Everything except the roots of those trees is hashed and
has ->mnt_child set up.
->mnt_hash of the roots of those copies host the cyclic list,
anchored in tree_list passed to propagate_mnt().
->mnt_list in each copy forms an unanchored cyclic list
going through all mounts in that copy.
The loop in attach_recursive_mnt() takes the tree_list apart and for each
element (== each copy of source tree) we have commit_tree() called once,
doing the remaining work:
* splices the ->mnt_list into the namespace's mount list
* sets ->mnt_ns for all nodes (root and submounts alike)
* sets ->mnt_child and ->mnt_hash for the root.
Again, the loop in attach_recursive_mnt() is over the set of secondary
copies of the source tree; it goes *only* through their roots. Submounts
are seen only by commit_tree(), in the list_for_each_entry() loop in
that function. Hell, try to add else WARN_ON(1); to that if (...) of yours
and see if you can trigger it if you don't believe the above...
> > You have "if mount is not locked - mark it; if mount is already marked -
> > mark it again". The latter part (|| IS_MNT_MARKED(mnt), that is) looks
> > very odd, won't you agree? What the hell was that (its counterpart in
> > the earlier code) about?
>
> Not mark it again. If the parent is marked mark the child.
*doh*
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
