On Thu, Jan 12, 2017 at 05:19:34AM +1300, Eric W. Biederman wrote:
> + if (child->mnt.mnt_root == smp->m_dentry) {
> + struct mount *q;
> + q = __lookup_mnt(&child->mnt_parent->mnt,
> + child->mnt_mountpoint);
> + if (q)
> + mnt_change_mountpoint(child, smp, q);
> + }
This is wrong; condition will be true for *all* mounts seen by that loop.
Feel free to add else WARN_ON(1); to the line above and try to trigger
it. You are misinterpreting what propagate_mnt() and commit_tree() are
doing - the loop in commit_tree() goes through the submounts and sets ->mnt_ns
on those. The of the fields is already set up by that point. For roots
of those copies we need to set ->mnt_hash/->mnt_child as well, but for
all submounts it's already been done by copy_tree(). Again, commit_tree()
is called once per secondary copy of source tree, not once per created
mount.
> +static struct mount *find_topper(struct mount *mnt)
> +{
> + /* If there is exactly one mount covering mnt completely return it. */
> + struct mount *child;
> +
> + if (!list_is_singular(&mnt->mnt_mounts))
> + return NULL;
> +
> + child = list_first_entry(&mnt->mnt_mounts, struct mount, mnt_child);
> + if (child->mnt_parent != mnt ||
The first part can't happen. Turn that into WARN_ON(child->mnt_parent != mnt)
if you wish, but that never occurs unless the data structures are corrupted.
> + child->mnt_mountpoint != mnt->mnt.mnt_root)
> + return NULL;
> @@ -342,7 +358,7 @@ static inline int do_refcount_check(struct mount *mnt, int count)
> */
> int propagate_mount_busy(struct mount *mnt, int refcnt)
> {
> - struct mount *m, *child;
> + struct mount *m, *child, *topper;
> struct mount *parent = mnt->mnt_parent;
>
> if (mnt == parent)
> @@ -358,11 +374,21 @@ int propagate_mount_busy(struct mount *mnt, int refcnt)
>
> for (m = propagation_next(parent, parent); m;
> m = propagation_next(m, parent)) {
> - child = __lookup_mnt_last(&m->mnt, mnt->mnt_mountpoint);
> + int count = 1;
> + child = __lookup_mnt(&m->mnt, mnt->mnt_mountpoint);
> if (!child)
> continue;
> - if (!list_empty(&child->mnt_mounts) ||
> - do_refcount_check(child, 1))
> +
> + /* Is there exactly one mount on the child that covers
> + * it completely whose reference should be ignored?
> + */
> + topper = find_topper(child);
> + if (topper)
> + count += 1;
> + else if (!list_empty(&child->mnt_mounts))
> + return 1;
> +
> + if (do_refcount_check(child, count))
> return 1;
Again, subject to the comments re semantics change (see the reply to previous
patch).
> @@ -431,6 +459,15 @@ static void __propagate_umount(struct mount *mnt)
> if (!child || !IS_MNT_MARKED(child))
> continue;
> CLEAR_MNT_MARK(child);
> +
> + /* If there is exactly one mount covering all of child
> + * replace child with that mount.
> + */
> + topper = find_topper(child);
> + if (topper)
> + mnt_change_mountpoint(child->mnt_parent, child->mnt_mp,
> + topper);
> +
> if (list_empty(&child->mnt_mounts)) {
> list_del_init(&child->mnt_child);
> child->mnt.mnt_flags |= MNT_UMOUNT;
Umm... With fallthrough from "is completely overmounted" case? And
I'm not sure I understand what that list_empty() is doing there after
your previous semantics change - how _can_ we reach that point with
non-empty ->mnt_mounts now?
--
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
