On Thu, Dec 12, 2024 at 01:09:29PM +0100, Peter Zijlstra wrote:
> On Thu, Dec 12, 2024 at 12:56:02PM +0100, Christian Brauner wrote:
>
>
> > @@ -16,7 +16,10 @@ struct mnt_namespace {
> > u64 event;
> > unsigned int nr_mounts; /* # of mounts in the namespace */
> > unsigned int pending_mounts;
> > - struct rb_node mnt_ns_tree_node; /* node in the mnt_ns_tree */
> > + union {
> > + struct rb_node mnt_ns_tree_node; /* node in the mnt_ns_tree */
> > + struct rcu_head mnt_ns_rcu;
> > + };
> > refcount_t passive; /* number references not pinning @mounts */
> > } __randomize_layout;
>
> > static void mnt_ns_tree_remove(struct mnt_namespace *ns)
> > {
> > /* remove from global mount namespace list */
> > if (!is_anon_ns(ns)) {
> > - guard(write_lock)(&mnt_ns_tree_lock);
> > + mnt_ns_tree_write_lock();
> > rb_erase(&ns->mnt_ns_tree_node, &mnt_ns_tree);
> > + mnt_ns_tree_write_unlock();
> > }
> >
> > - mnt_ns_release(ns);
> > + call_rcu(&ns->mnt_ns_rcu, mnt_ns_release_rcu);
> > }
>
> I'm not sure that union is sane; the above means you're overwriting the
> tree node while a concurrent lookup might still see the node and want to
> decent from it.
Though, you're right. I'll fix this up. Thanks for the reviews!
