linux-kernel - Re: [RFC v3 1/1] fs/namespace: remove RCU sync for MNT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <50512ec3-da6d-4140-9659-58e0514a4970@redhat.com>
Date: Thu, 27 Jun 2024 09:11:14 +0800
From: Ian Kent <ikent@...hat.com>
To: Matthew Wilcox <willy@...radead.org>,
 Lucas Karpinski <lkarpins@...hat.com>
Cc: viro@...iv.linux.org.uk, brauner@...nel.org, jack@...e.cz,
 raven@...maw.net, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, Alexander Larsson <alexl@...hat.com>,
 Eric Chanudet <echanude@...hat.com>
Subject: Re: [RFC v3 1/1] fs/namespace: remove RCU sync for MNT_DETACH umount

On 27/6/24 04:47, Matthew Wilcox wrote:
> On Wed, Jun 26, 2024 at 04:07:49PM -0400, Lucas Karpinski wrote:
>> +++ b/fs/namespace.c
>> @@ -78,6 +78,7 @@ static struct kmem_cache *mnt_cache __ro_after_init;
>>   static DECLARE_RWSEM(namespace_sem);
>>   static HLIST_HEAD(unmounted);	/* protected by namespace_sem */
>>   static LIST_HEAD(ex_mountpoints); /* protected by namespace_sem */
>> +static bool lazy_unlock = false; /* protected by namespace_sem */
> That's a pretty ugly way of doing it.  How about this?

Ha!

That was my original thought but I also didn't much like changing all 
the callers.

I don't really like the proliferation of these small helper functions 
either but if everyone

is happy to do this I think it's a great idea.


Ian

>
> +++ b/fs/namespace.c
> @@ -1553,7 +1553,7 @@ int may_umount(struct vfsmount *mnt)
>   
>   EXPORT_SYMBOL(may_umount);
>   
> -static void namespace_unlock(void)
> +static void __namespace_unlock(bool lazy)
>   {
>          struct hlist_head head;
>          struct hlist_node *p;
> @@ -1570,7 +1570,8 @@ static void namespace_unlock(void)
>          if (likely(hlist_empty(&head)))
>                  return;
>   
> -       synchronize_rcu_expedited();
> +       if (!lazy)
> +               synchronize_rcu_expedited();
>   
>          hlist_for_each_entry_safe(m, p, &head, mnt_umount) {
>                  hlist_del(&m->mnt_umount);
> @@ -1578,6 +1579,11 @@ static void namespace_unlock(void)
>          }
>   }
>   
> +static inline void namespace_unlock(void)
> +{
> +       __namespace_unlock(false);
> +}
> +
>   static inline void namespace_lock(void)
>   {
>          down_write(&namespace_sem);
> @@ -1798,7 +1804,7 @@ static int do_umount(struct mount *mnt, int flags)
>          }
>   out:
>          unlock_mount_hash();
> -       namespace_unlock();
> +       __namespace_unlock(flags & MNT_DETACH);
>          return retval;
>   }
>   
>
> (other variants on this theme might be to pass the flags to
> __namespace_unlock() and check MNT_DETACH there)
>