Re: [git pull] pile 1: mount stuff

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

On Fri, Oct 03, 2025 at 10:45:02AM -0700, Linus Torvalds wrote:

> What do you think? This patch is ENTIRELY UNTESTED. It compiled for
> me, that's all I can say. It *looks* fine, and it reads nicely and
> groups those variables with the lock that protects them. But it's a
> quick throw-away patch for "maybe something like this?"

Gathering those into a single object makes sense, the only thing that
worries me is that it is suggesting that we might want to have multiple
instances of the entire thing...

One obvious problem is notify_list and mnt_notify_add(); it's not
impossible to deal with.  The only reason it's not static is the
call in reparent(), and I'm not at all sure it needs to be there -
we are sliding something from under an overmount and I don't see
why would anybody want a notification for watchers of that
overmount, especially since nothing is generated for the things
overmounting it, etc.

Last cycle's pile had been way too large as it is, so I decided to
leave dealign with that magical mystery shite for later; might as well
do it this cycle...

>  /*
> - * locks: mount_lock [read_seqlock_excl], namespace_sem [excl]
> + * locks: mount_lock [read_seqlock_excl], fs_namespace.sem [excl]

 * locks: mount_locked_reader, namespace_excl

would be better, IMO.  I didn't switch those comments since the series had
already been getting too long, but I think refering to locking conditions
by guard names is better than going by lock name + type of access.

> -/* iterator; we want it to have access to namespace_sem, thus here... */
> +/* iterator; we want it to have access to fs_namespace.sem, thus here... */

Stale comment, really - mnt_find_id_at() is static, to start with...
Originally it had been explaining why that thing (used in fs/proc_namespace.c)
stayed behind in fs/namespace.c; these days it's not obvious that comment is
needed in the first place, but if we want to explain that, we would be better
off with something like "iterator; used only by fs/proc_namespace.c, kept here
since it uses mount rbtree of namespace and we don't want to expose the details
of that outside of fs/namespace.c"...

> -		emptied_ns = m->mnt_ns;
> +		fs_namespace.emptied_ns = m->mnt_ns;

Might be worth an inlined helper...

> -	rwsem_assert_held(&namespace_sem);
> +	rwsem_assert_held(&fs_namespace.sem);

... as well as this one, really.