Re: [PATCH 2/3] namespace: rework connected mounts

[Al Viro <viro@zeniv.linux.org.uk>]

On Sun, Oct 18, 2026 at 06:32:19PM +0200, Christian Brauner wrote:

> Sure. Note that it's a separate allocation so 40 bytes land in
> kmalloc-64 just like 64 do. IOW, it's a cleanup, not a saving. I can do
> that for 7.4.

FWIW, I really don't like that separate allocation ;-/  For the majority
of struct mount instances we never access that thing after it's been set
up...

Sure, I see your point re separating the cleanup_mnt() guts and
detaching from the parent, and yes, having two separate structures
is a simple way to achieve that.  However, your variant pays for
that with overhead and I'm not convinced that it's actually simpler.

Consider the situation when mount reaches
	call_rcu(&mnt->mnt_rcu, delayed_free_vfsmnt);
in the end of cleanup_mnt().  Possible states:

1) never called leave_cover() at all.
2) called leave_cover(), matching drop_cover() has already happened.
3) called leave_cover(), matching drop_cover() is yet to come.

(1) is by far the most likely.  Suppose we have leave_cover() set
a new flag (MNT_COVER) and _not_ clear ->mnt_cover; that happens
under mount_lock and always ordered before cleanup_mnt().
Then we can have cleanup_mnt() do the following:

	if no MNT_COVER		// case 1
		call_rcu(...)
	else
		read_seqlock_excl(&mount_lock);
		set MNT_MAY_FREE
		if cover is already off the lists	// case 2
			call_rcu(...)
		read_sequnlock_excl(&mount_lock);

and in drop_cover()
	remove from lists
	if MNT_MAY_FREE		// case 3
		call_rcu(...)
with delayed_free_vfsmnt() doing all freeing.  But then we don't
need the separate allocation at all.  What's more, we can reuse
->mnt_mp for ->mnt_cover->mp and ->mnt_mp_list for ->mnt_cover->pin
just by having __umount_mnt() make
	hlist_del_init(&mnt->mnt_mp_list);
	mp = mnt->mnt_mp;   
	mnt->mnt_mp = NULL;  
	maybe_free_mountpoint(mp, shrink_list);
conditional upon the lack of MNT_COVER.  That kills your extra
check in maybe_free_mountpoint(), with the loops in __detach_mounts()
folded together (they'd walk the same list, using MNT_COVER to
discriminate between umount_tree() and drop_cover()).

->cover->node moves to mount itself; it would be tempting to union
it with mnt_hash (making removal from mnt_hash in __umount_mnt()
also conditional upon !MNT_COVER).  _Probably_ safe - callers of
__lookup_mnt() either have mount_lock or check the seqcount side
of things soon enough.  Will need to check how that affects the
proof of safety in your "skip synchronize_rcu if possible" series,
though...

->cover->dentry...  Fuck knows - might union with ->mnt_mountpoint
(again, conditional upon MNT_COVER), might put it separately;
->mnt_cover goes away, so the size of struct mount won't grow.
Bits in ->mnt_flags are not scarce these days - we can afford stealing
two easily...

I'll try to cook something along those lines; looks at least plausible...