[2/2] kobject: reduce uevent_sock_mutex scope

Message ID 20240214084829.684541-3-edumazet@google.com
State New
Headers
Series kobject: reduce uevent_sock_mutex contention |

Commit Message

Eric Dumazet Feb. 14, 2024, 8:48 a.m. UTC
  This is a followup of commit a3498436b3a0 ("netns: restrict uevents")

- uevent_sock_mutex no longer protects uevent_seqnum thanks
  to prior patch in the series.

- uevent_net_broadcast() can run without holding uevent_sock_mutex.

- Instead of grabbing uevent_sock_mutex before calling
  kobject_uevent_net_broadcast(), we can move the
  mutex_lock(&uevent_sock_mutex) to the place we iterate over
  uevent_sock_list : uevent_net_broadcast_untagged().

After this patch, typical netdevice creations and destructions
calling uevent_net_broadcast_tagged() no longer need to acquire
uevent_sock_mutex.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Christian Brauner <brauner@kernel.org>
---
 lib/kobject_uevent.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
  

Comments

Christian Brauner Feb. 16, 2024, 2:53 p.m. UTC | #1
On Wed, Feb 14, 2024 at 08:48:29AM +0000, Eric Dumazet wrote:
> This is a followup of commit a3498436b3a0 ("netns: restrict uevents")
> 
> - uevent_sock_mutex no longer protects uevent_seqnum thanks
>   to prior patch in the series.
> 
> - uevent_net_broadcast() can run without holding uevent_sock_mutex.
> 
> - Instead of grabbing uevent_sock_mutex before calling
>   kobject_uevent_net_broadcast(), we can move the
>   mutex_lock(&uevent_sock_mutex) to the place we iterate over
>   uevent_sock_list : uevent_net_broadcast_untagged().
> 
> After this patch, typical netdevice creations and destructions
> calling uevent_net_broadcast_tagged() no longer need to acquire
> uevent_sock_mutex.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Christian Brauner <brauner@kernel.org>
> ---

Very nice,
Reviewed-by: Christian Brauner <brauner@kernel.org>
  

Patch

diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index 9cb1a7fdaeba4fc5c698fbe84f359fb305345be1..03b427e2707e357ab12abeb9da234432c4bc0fb3 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -42,10 +42,9 @@  struct uevent_sock {
 
 #ifdef CONFIG_NET
 static LIST_HEAD(uevent_sock_list);
-#endif
-
 /* This lock protects uevent_sock_list */
 static DEFINE_MUTEX(uevent_sock_mutex);
+#endif
 
 /* the strings here must match the enum in include/linux/kobject.h */
 static const char *kobject_actions[] = {
@@ -315,6 +314,7 @@  static int uevent_net_broadcast_untagged(struct kobj_uevent_env *env,
 	int retval = 0;
 
 	/* send netlink message */
+	mutex_lock(&uevent_sock_mutex);
 	list_for_each_entry(ue_sk, &uevent_sock_list, list) {
 		struct sock *uevent_sock = ue_sk->sk;
 
@@ -334,6 +334,7 @@  static int uevent_net_broadcast_untagged(struct kobj_uevent_env *env,
 		if (retval == -ENOBUFS || retval == -ESRCH)
 			retval = 0;
 	}
+	mutex_unlock(&uevent_sock_mutex);
 	consume_skb(skb);
 
 	return retval;
@@ -589,10 +590,8 @@  int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
 	if (retval)
 		goto exit;
 
-	mutex_lock(&uevent_sock_mutex);
 	retval = kobject_uevent_net_broadcast(kobj, env, action_string,
 					      devpath);
-	mutex_unlock(&uevent_sock_mutex);
 
 #ifdef CONFIG_UEVENT_HELPER
 	/* call uevent_helper, usually only enabled during early boot */
@@ -743,9 +742,7 @@  static int uevent_net_rcv_skb(struct sk_buff *skb, struct nlmsghdr *nlh,
 		return -EPERM;
 	}
 
-	mutex_lock(&uevent_sock_mutex);
 	ret = uevent_net_broadcast(net->uevent_sock->sk, skb, extack);
-	mutex_unlock(&uevent_sock_mutex);
 
 	return ret;
 }