For each removed mount we need to calculate where the slaves will end up.
To avoid duplicating that work, do it for all mounts to be removed
at once, taking the mounts themselves out of propagation graph as
we go, then do all transfers; the duplicate work on finding destinations
is avoided since if we run into a mount that already had destination found,
we don't need to trace the rest of the way. That's guaranteed
O(removed mounts) for finding destinations and removing from propagation
graph and O(surviving mounts that have master removed) for transfers.
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
... and document that locking requirements for is_path_reachable().
There is one questionable caller in do_listmount() where we are not
holding mount_lock *and* might not have the first argument mounted.
However, in that case it will immediately return true without having
to look at the ancestors. Might be cleaner to move the check into
non-LSTM_ROOT case which it really belongs in - there the check is
not always true and is_mounted() is guaranteed.
Document the locking environments for is_path_reachable() callers:
get_peer_under_root()
get_dominating_id()
do_statmount()
do_listmount()
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
We only need it when mount in question was sending events downstream (then
recepients need to switch to new master) or the mount is being turned into
slave (then we need a new master for it).
That wouldn't be a big deal, except that it causes quite a bit of work
when umount_tree() is taking a large peer group out. Adding a trivial
"don't bother calling propagation_source() unless we are going to use
its results" logics improves the things quite a bit.
We are still doing unnecessary work on bulk removals from propagation graph,
but the full solution for that will have to wait for the next merge window.
Fixes: 955336e204 "do_make_slave(): choose new master sanely"
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
... as the comments in reparent() clearly say. As it is, we reparent
*all* overmounts of the mounts being taken out, including those that
are taken out themselves. It's not only a potentially massive slowdown
(on a pathological setup we might end up with O(N^2) time for N mounts
being kicked out), it can end up with incorrect ->overmount in the
surviving mounts.
Fixes: f0d0ba1998 "Rewrite of propagate_umount()"
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
The only place that really needs to be adjusted is commit_tree() -
there we need to iterate through the copy and we might as well
use next_mnt() for that. However, in case when our tree has been
slid under something already mounted (propagation to a mountpoint
that already has something mounted on it or a 'beneath' move_mount)
we need to take care not to walk into the overmounting tree.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Lift calculation of replacement propagation source, removal from
peer group and assignment of ->mnt_master from do_make_slave() into
change_mnt_propagation() itself. What remains is switching of
what used to get propagation *through* mnt to alternative source.
Rename to transfer_propagation(), passing it the replacement source
as the second argument. Have it return void, while we are at it.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
When mount changes propagation type so that it doesn't propagate
events any more (MS_PRIVATE, MS_SLAVE, MS_UNBINDABLE), we need
to make sure that event propagation between other mounts is
unaffected.
We need to make sure that events from peers and master of that mount
(if any) still reach everything that used to be on its ->mnt_slave_list.
If mount has neither peers nor master, we simply need to dissolve
its ->mnt_slave_list and clear ->mnt_master of everything in there.
If mount has peers, we transfer everything in ->mnt_slave_list of
this mount into that of some of those peers (and adjust ->mnt_master
accordingly).
If mount has a master but no peers, we transfer everything in
->mnt_slave_list of this mount into that of its master (adjusting
->mnt_master, etc.).
There are two problems with the current implementation:
* there's a long-obsolete logics in choosing the peer -
once upon a time it made sense to prefer the peer that had the
same ->mnt_root as our mount, but that had been pointless since
2014 ("smarter propagate_mnt()")
* the most common caller of that thing is umount_tree()
taking the mounts out of propagation graph. In that case it's
possible to have ->mnt_slave_list contents moved many times,
since the replacement master is likely to be taken out by the
same umount_tree(), etc.
Take the choice of replacement master into a separate function
(propagation_source()) and teach it to skip the candidates that
are going to be taken out.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
... since mnt->mnt_share and mnt->mnt_slave_list are guaranteed to be empty unless
IS_MNT_SHARED(mnt).
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Mountpoint is passed as struct mountpoint *, not struct dentry *
(and called dest_mp, not dest_dentry) since 2013.
Roots of created copies are linked via mnt_hash, not mnt_list since
a bit before the merge into mainline back in 2005.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Its only use is choosing the type of copy - CL_MAKE_SHARED if there
already is a copy in that peer group, CL_SLAVE or CL_SLAVE | CL_MAKE_SHARED
otherwise.
But that's easy to keep track of - just set type in the beginning of group
and reset to CL_MAKE_SHARED after the first created secondary in it...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
this stuff can be local in propagate_mnt() now (and in some cases
duplicates the existing variables there)
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
When we create the first copy for a peer group, it becomes a slave of
one of the existing copies; take that logics into a separate helper -
find_master(parent, last_copy, original).
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
the only difference is that for the original group we want to skip
the first element; not worth having the logics twice...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
propagate_mnt() takes the subtree we are about to attach and creates
its copies, setting the propagation between those. Each copy is cloned
either from the original or from one of the already created copies.
The tricky part is choosing the right copy to serve as a master when we
are starting a new peer group.
The algorithm for doing that selection puts temporary marks on the masters
of mountpoints that already got a copy created for them; since the initial
peer group might have no master at all, we need to special-case that when
looking for the mark. Currently we do that by memorizing the master of
original peer group. It works, but we get yet another piece of data to
pass from propagate_mnt() to propagate_one().
Alternative is to mark the master of original peer group if not NULL,
turning the check into "master is NULL or marked". Less data to pass
around and memory safety is more obvious that way...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Several flags are updated and checked only under namespace_sem; we are
already making use of that when we are checking them without mount_lock,
but we have to hold mount_lock for all updates, which makes things
clumsier than they have to be.
Take MNT_SHARED, MNT_UNBINDABLE, MNT_MARKED and MNT_UMOUNT_CANDIDATE
into a separate field (->mnt_t_flags), renaming them to T_SHARED,
etc. to avoid confusion. All accesses must be under namespace_sem.
That changes locking requirements for mnt_change_propagation() and
set_mnt_shared() - only namespace_sem is needed now. The same goes
for SET_MNT_MARKED et.al.
There might be more flags moved from ->mnt_flags to that field;
this is just the initial set.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Simplify the rules for mount refcounts. Current rules include:
* being a namespace root => +1
* being someone's child => +1
* being someone's child => +1 to parent's refcount, unless you've
already been through umount_tree().
The last part is not needed at all. It makes for more places where need
to decrement refcounts and it creates an asymmetry between the situations
for something that has never been a part of a namespace and something that
left one, both for no good reason.
If mount's refcount has additions from its children, we know that
* it's either someone's child itself (and will remain so
until umount_tree(), at which point contributions from children
will disappear), or
* or is the root of namespace (and will remain such until
it either becomes someone's child in another namespace or goes through
umount_tree()), or
* it is the root of some tree copy, and is currently pinned
by the caller of copy_tree() (and remains such until it either gets
into namespace, or goes to umount_tree()).
In all cases we already have contribution(s) to refcount that will last
as long as the contribution from children remains. In other words, the
lifetime is not affected by refcount contributions from children.
It might be useful for "is it busy" checks, but those are actually
no harder to express without it.
NB: propagate_mnt_busy() part is an equivalent transformation, ugly as it
is; the current logics is actually wrong and may give false negatives,
but fixing that is for a separate patch (probably earlier in the queue).
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
The variant currently in the tree has problems; trying to prove
correctness has caught at least one class of bugs (reparenting
that ends up moving the visible location of reparented mount, due
to not excluding some of the counterparts on propagation that
should've been included).
I tried to prove that it's the only bug there; I'm still not sure
whether it is. If anyone can reconstruct and write down an analysis
of the mainline implementation, I'll gladly review it; as it is,
I ended up doing a different implementation. Candidate collection
phase is similar, but trimming the set down until it satisfies the
constraints turned out pretty different.
I hoped to do transformation as a massage series, but that turns out
to be too convoluted. So it's a single patch replacing propagate_umount()
and friends in one go, with notes and analysis in D/f/propagate_umount.txt
(in addition to inline comments).
As far I can tell, it is provably correct and provably linear by the number
of mounts we need to look at in order to decide what should be unmounted.
It even builds and seems to survive testing...
Another nice thing that fell out of that is that ->mnt_umounting is no longer
needed.
Compared to the first version:
* explicit MNT_UMOUNT_CANDIDATE flag for is_candidate()
* trim_ancestors() only clears that flag, leaving the suckers on list
* trim_one() and handle_locked() take the stuff with flag cleared off
the list. That allows to iterate with list_for_each_entry_safe() when calling
trim_one() - it removes at most one element from the list now.
* no globals - I didn't bother with any kind of context, not worth it.
* Notes updated accordingly; I have not touch the terms yet.
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
All versions up to 6.14 did not propagate mount events into detached
tree. Shortly after 6.14 a merge of vfs-6.15-rc1.mount.namespace
(130e696aa6) has changed that.
Unfortunately, that has caused userland regressions (reported in
https://lore.kernel.org/all/CAOYeF9WQhFDe+BGW=Dp5fK8oRy5AgZ6zokVyTj1Wp4EUiYgt4w@mail.gmail.com/)
Straight revert wouldn't be an option - in particular, the variant in 6.14
had a bug that got fixed in d1ddc6f1d9 ("fix IS_MNT_PROPAGATING uses")
and we don't want to bring the bug back.
This is a modification of manual revert posted by Christian, with changes
needed to avoid reintroducing the breakage in scenario described in
d1ddc6f1d9.
Cc: stable@vger.kernel.org
Reported-by: Allison Karlitskaya <lis@redhat.com>
Tested-by: Allison Karlitskaya <lis@redhat.com>
Acked-by: Christian Brauner <brauner@kernel.org>
Co-developed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Al Viro