diff mbox series

[v2,net-next,6/6] af_unix: Remove unix_table_locks.

Message ID 20220620185151.65294-7-kuniyu@amazon.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series af_unix: Introduce per-netns socket hash table. | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 58 this patch: 61
netdev/cc_maintainers warning 1 maintainers not CCed: viro@zeniv.linux.org.uk
netdev/build_clang fail Errors and warnings before: 7 this patch: 7
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 59 this patch: 62
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 173 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kuniyuki Iwashima June 20, 2022, 6:51 p.m. UTC 
unix_table_locks are to protect the global hash table, unix_socket_table.
The previous commit removed it, so let's clean up the unnecessary locks.

Here is a test result on EC2 c5.9xlarge where 10 processes run concurrently
in different netns and bind 100,000 sockets for each.

  without this series : 1m 38s
  with this series    :    11s

It is ~10x faster because the global hash table is split into 10 netns in
this case.

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 include/net/af_unix.h |  1 -
 net/unix/af_unix.c    | 30 ++----------------------------
 net/unix/diag.c       |  5 -----
 3 files changed, 2 insertions(+), 34 deletions(-)

Comments

kernel test robot June 20, 2022, 9:39 p.m. UTC | #1 
Hi Kuniyuki,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/af_unix-Introduce-per-netns-socket-hash-table/20220621-025503
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git dbca1596bbb08318f5e3b3b99f8ca0a0d3830a65
config: arc-defconfig (https://download.01.org/0day-ci/archive/20220621/202206210532.q32y1W52-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/f2cd3aee6929543114a0728717969d9bb2f6fa90
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kuniyuki-Iwashima/af_unix-Introduce-per-netns-socket-hash-table/20220621-025503
        git checkout f2cd3aee6929543114a0728717969d9bb2f6fa90
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   net/unix/diag.c: In function 'unix_lookup_by_ino':
>> net/unix/diag.c:247:46: error: 'unix_table_locks' undeclared (first use in this function); did you mean 'unix_state_lock'?
     247 |                                 spin_unlock(&unix_table_locks[i]);
         |                                              ^~~~~~~~~~~~~~~~
         |                                              unix_state_lock
   net/unix/diag.c:247:46: note: each undeclared identifier is reported only once for each function it appears in


vim +247 net/unix/diag.c

22931d3b906cd0 Pavel Emelyanov   2011-12-15  235  
a50feb1de03567 Kuniyuki Iwashima 2022-06-20  236  static struct sock *unix_lookup_by_ino(struct net *net, unsigned int ino)
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  237  {
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  238  	struct sock *sk;
afd20b9290e184 Kuniyuki Iwashima 2021-11-24  239  	int i;
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  240  
614d83f20d005b Kuniyuki Iwashima 2022-06-20  241  	for (i = 0; i < UNIX_HASH_SIZE; i++) {
a50feb1de03567 Kuniyuki Iwashima 2022-06-20  242  		spin_lock(&net->unx.table.locks[i]);
868a38bbd9ad2a Kuniyuki Iwashima 2022-06-20  243  		sk_for_each(sk, &net->unx.table.buckets[i]) {
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  244  			if (ino == sock_i_ino(sk)) {
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  245  				sock_hold(sk);
a50feb1de03567 Kuniyuki Iwashima 2022-06-20  246  				spin_unlock(&net->unx.table.locks[i]);
afd20b9290e184 Kuniyuki Iwashima 2021-11-24 @247  				spin_unlock(&unix_table_locks[i]);
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  248  				return sk;
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  249  			}
868a38bbd9ad2a Kuniyuki Iwashima 2022-06-20  250  		}
a50feb1de03567 Kuniyuki Iwashima 2022-06-20  251  		spin_unlock(&net->unx.table.locks[i]);
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  252  	}
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  253  	return NULL;
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  254  }
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  255
kernel test robot June 20, 2022, 9:39 p.m. UTC | #2 
Hi Kuniyuki,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/af_unix-Introduce-per-netns-socket-hash-table/20220621-025503
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git dbca1596bbb08318f5e3b3b99f8ca0a0d3830a65
config: i386-randconfig-a012-20220620 (https://download.01.org/0day-ci/archive/20220621/202206210554.YYgdQz8e-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project af6d2a0b6825e71965f3e2701a63c239fa0ad70f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/f2cd3aee6929543114a0728717969d9bb2f6fa90
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kuniyuki-Iwashima/af_unix-Introduce-per-netns-socket-hash-table/20220621-025503
        git checkout f2cd3aee6929543114a0728717969d9bb2f6fa90
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash net/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> net/unix/diag.c:247:18: error: use of undeclared identifier 'unix_table_locks'
                                   spin_unlock(&unix_table_locks[i]);
                                                ^
   1 error generated.


vim +/unix_table_locks +247 net/unix/diag.c

22931d3b906cd0 Pavel Emelyanov   2011-12-15  235  
a50feb1de03567 Kuniyuki Iwashima 2022-06-20  236  static struct sock *unix_lookup_by_ino(struct net *net, unsigned int ino)
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  237  {
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  238  	struct sock *sk;
afd20b9290e184 Kuniyuki Iwashima 2021-11-24  239  	int i;
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  240  
614d83f20d005b Kuniyuki Iwashima 2022-06-20  241  	for (i = 0; i < UNIX_HASH_SIZE; i++) {
a50feb1de03567 Kuniyuki Iwashima 2022-06-20  242  		spin_lock(&net->unx.table.locks[i]);
868a38bbd9ad2a Kuniyuki Iwashima 2022-06-20  243  		sk_for_each(sk, &net->unx.table.buckets[i]) {
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  244  			if (ino == sock_i_ino(sk)) {
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  245  				sock_hold(sk);
a50feb1de03567 Kuniyuki Iwashima 2022-06-20  246  				spin_unlock(&net->unx.table.locks[i]);
afd20b9290e184 Kuniyuki Iwashima 2021-11-24 @247  				spin_unlock(&unix_table_locks[i]);
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  248  				return sk;
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  249  			}
868a38bbd9ad2a Kuniyuki Iwashima 2022-06-20  250  		}
a50feb1de03567 Kuniyuki Iwashima 2022-06-20  251  		spin_unlock(&net->unx.table.locks[i]);
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  252  	}
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  253  	return NULL;
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  254  }
5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  255
Kuniyuki Iwashima June 20, 2022, 9:53 p.m. UTC | #3 
From:   kernel test robot <lkp@intel.com>
Date:   Tue, 21 Jun 2022 05:39:37 +0800
> Hi Kuniyuki,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on net-next/master]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/af_unix-Introduce-per-netns-socket-hash-table/20220621-025503
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git dbca1596bbb08318f5e3b3b99f8ca0a0d3830a65
> config: arc-defconfig (https://download.01.org/0day-ci/archive/20220621/202206210532.q32y1W52-lkp@intel.com/config)
> compiler: arc-elf-gcc (GCC) 11.3.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/f2cd3aee6929543114a0728717969d9bb2f6fa90
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Kuniyuki-Iwashima/af_unix-Introduce-per-netns-socket-hash-table/20220621-025503
>         git checkout f2cd3aee6929543114a0728717969d9bb2f6fa90
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    net/unix/diag.c: In function 'unix_lookup_by_ino':
> >> net/unix/diag.c:247:46: error: 'unix_table_locks' undeclared (first use in this function); did you mean 'unix_state_lock'?
>      247 |                                 spin_unlock(&unix_table_locks[i]);
>          |                                              ^~~~~~~~~~~~~~~~
>          |                                              unix_state_lock
>    net/unix/diag.c:247:46: note: each undeclared identifier is reported only once for each function it appears in

I'm sorry, CONFIG_UNIX_DIAG was not set in my .config.
I'll respin v3.

Best regards,
Kuniyuki


> vim +247 net/unix/diag.c
> 
> 22931d3b906cd0 Pavel Emelyanov   2011-12-15  235  
> a50feb1de03567 Kuniyuki Iwashima 2022-06-20  236  static struct sock *unix_lookup_by_ino(struct net *net, unsigned int ino)
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  237  {
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  238  	struct sock *sk;
> afd20b9290e184 Kuniyuki Iwashima 2021-11-24  239  	int i;
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  240  
> 614d83f20d005b Kuniyuki Iwashima 2022-06-20  241  	for (i = 0; i < UNIX_HASH_SIZE; i++) {
> a50feb1de03567 Kuniyuki Iwashima 2022-06-20  242  		spin_lock(&net->unx.table.locks[i]);
> 868a38bbd9ad2a Kuniyuki Iwashima 2022-06-20  243  		sk_for_each(sk, &net->unx.table.buckets[i]) {
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  244  			if (ino == sock_i_ino(sk)) {
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  245  				sock_hold(sk);
> a50feb1de03567 Kuniyuki Iwashima 2022-06-20  246  				spin_unlock(&net->unx.table.locks[i]);
> afd20b9290e184 Kuniyuki Iwashima 2021-11-24 @247  				spin_unlock(&unix_table_locks[i]);
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  248  				return sk;
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  249  			}
> 868a38bbd9ad2a Kuniyuki Iwashima 2022-06-20  250  		}
> a50feb1de03567 Kuniyuki Iwashima 2022-06-20  251  		spin_unlock(&net->unx.table.locks[i]);
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  252  	}
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  253  	return NULL;
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  254  }
> 5d3cae8bc39dd3 Pavel Emelyanov   2011-12-15  255  
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
diff mbox series

Patch

diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index b1748c9b6db2..480fa579787e 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -21,7 +21,6 @@  struct sock *unix_peer_get(struct sock *sk);
 #define UNIX_HASH_BITS	8
 
 extern unsigned int unix_tot_inflight;
-extern spinlock_t unix_table_locks[UNIX_HASH_SIZE];
 
 struct unix_address {
 	refcount_t	refcnt;
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 9d0b07235dbc..eaf5b5b429c1 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -118,8 +118,6 @@ 
 
 #include "scm.h"
 
-spinlock_t unix_table_locks[UNIX_HASH_SIZE];
-EXPORT_SYMBOL_GPL(unix_table_locks);
 static atomic_long_t unix_nr_socks;
 
 /* SMP locking strategy:
@@ -166,9 +164,6 @@  static void unix_table_double_lock(struct net *net,
 	if (hash1 > hash2)
 		swap(hash1, hash2);
 
-	spin_lock(&unix_table_locks[hash1]);
-	spin_lock_nested(&unix_table_locks[hash2], SINGLE_DEPTH_NESTING);
-
 	spin_lock(&net->unx.table.locks[hash1]);
 	spin_lock_nested(&net->unx.table.locks[hash2], SINGLE_DEPTH_NESTING);
 }
@@ -178,9 +173,6 @@  static void unix_table_double_unlock(struct net *net,
 {
 	spin_unlock(&net->unx.table.locks[hash1]);
 	spin_unlock(&net->unx.table.locks[hash2]);
-
-	spin_unlock(&unix_table_locks[hash1]);
-	spin_unlock(&unix_table_locks[hash2]);
 }
 
 #ifdef CONFIG_SECURITY_NETWORK
@@ -324,20 +316,16 @@  static void __unix_set_addr_hash(struct net *net, struct sock *sk,
 
 static void unix_remove_socket(struct net *net, struct sock *sk)
 {
-	spin_lock(&unix_table_locks[sk->sk_hash]);
 	spin_lock(&net->unx.table.locks[sk->sk_hash]);
 	__unix_remove_socket(sk);
 	spin_unlock(&net->unx.table.locks[sk->sk_hash]);
-	spin_unlock(&unix_table_locks[sk->sk_hash]);
 }
 
 static void unix_insert_unbound_socket(struct net *net, struct sock *sk)
 {
-	spin_lock(&unix_table_locks[sk->sk_hash]);
 	spin_lock(&net->unx.table.locks[sk->sk_hash]);
 	__unix_insert_socket(net, sk);
 	spin_unlock(&net->unx.table.locks[sk->sk_hash]);
-	spin_unlock(&unix_table_locks[sk->sk_hash]);
 }
 
 static struct sock *__unix_find_socket_byname(struct net *net,
@@ -362,13 +350,11 @@  static inline struct sock *unix_find_socket_byname(struct net *net,
 {
 	struct sock *s;
 
-	spin_lock(&unix_table_locks[hash]);
 	spin_lock(&net->unx.table.locks[hash]);
 	s = __unix_find_socket_byname(net, sunname, len, hash);
 	if (s)
 		sock_hold(s);
 	spin_unlock(&net->unx.table.locks[hash]);
-	spin_unlock(&unix_table_locks[hash]);
 	return s;
 }
 
@@ -377,7 +363,6 @@  static struct sock *unix_find_socket_byinode(struct net *net, struct inode *i)
 	unsigned int hash = unix_bsd_hash(i);
 	struct sock *s;
 
-	spin_lock(&unix_table_locks[hash]);
 	spin_lock(&net->unx.table.locks[hash]);
 	sk_for_each(s, &net->unx.table.buckets[hash]) {
 		struct dentry *dentry = unix_sk(s)->path.dentry;
@@ -385,12 +370,10 @@  static struct sock *unix_find_socket_byinode(struct net *net, struct inode *i)
 		if (dentry && d_backing_inode(dentry) == i) {
 			sock_hold(s);
 			spin_unlock(&net->unx.table.locks[hash]);
-			spin_unlock(&unix_table_locks[hash]);
 			return s;
 		}
 	}
 	spin_unlock(&net->unx.table.locks[hash]);
-	spin_unlock(&unix_table_locks[hash]);
 	return NULL;
 }
 
@@ -3253,7 +3236,6 @@  static struct sock *unix_get_first(struct seq_file *seq, loff_t *pos)
 	struct sock *sk;
 
 	while (bucket < UNIX_HASH_SIZE) {
-		spin_lock(&unix_table_locks[bucket]);
 		spin_lock(&net->unx.table.locks[bucket]);
 
 		sk = unix_from_bucket(seq, pos);
@@ -3261,7 +3243,6 @@  static struct sock *unix_get_first(struct seq_file *seq, loff_t *pos)
 			return sk;
 
 		spin_unlock(&net->unx.table.locks[bucket]);
-		spin_unlock(&unix_table_locks[bucket]);
 
 		*pos = set_bucket_offset(++bucket, 1);
 	}
@@ -3280,7 +3261,6 @@  static struct sock *unix_get_next(struct seq_file *seq, struct sock *sk,
 
 
 	spin_unlock(&seq_file_net(seq)->unx.table.locks[bucket]);
-	spin_unlock(&unix_table_locks[bucket]);
 
 	*pos = set_bucket_offset(++bucket, 1);
 
@@ -3309,10 +3289,8 @@  static void unix_seq_stop(struct seq_file *seq, void *v)
 {
 	struct sock *sk = v;
 
-	if (sk) {
+	if (sk)
 		spin_unlock(&seq_file_net(seq)->unx.table.locks[sk->sk_hash]);
-		spin_unlock(&unix_table_locks[sk->sk_hash]);
-	}
 }
 
 static int unix_seq_show(struct seq_file *seq, void *v)
@@ -3416,7 +3394,6 @@  static int bpf_iter_unix_hold_batch(struct seq_file *seq, struct sock *start_sk)
 	}
 
 	spin_unlock(&seq_file_net(seq)->unx.table.locks[start_sk->sk_hash]);
-	spin_unlock(&unix_table_locks[start_sk->sk_hash]);
 
 	return expected;
 }
@@ -3705,13 +3682,10 @@  static void __init bpf_iter_register(void)
 
 static int __init af_unix_init(void)
 {
-	int i, rc = -1;
+	int rc = -1;
 
 	BUILD_BUG_ON(sizeof(struct unix_skb_parms) > sizeof_field(struct sk_buff, cb));
 
-	for (i = 0; i < UNIX_HASH_SIZE; i++)
-		spin_lock_init(&unix_table_locks[i]);
-
 	rc = proto_register(&unix_dgram_proto, 1);
 	if (rc != 0) {
 		pr_crit("%s: Cannot create unix_sock SLAB cache!\n", __func__);
diff --git a/net/unix/diag.c b/net/unix/diag.c
index 4d0f0ca6a1eb..e7f5636daa68 100644
--- a/net/unix/diag.c
+++ b/net/unix/diag.c
@@ -208,7 +208,6 @@  static int unix_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
 		struct sock *sk;
 
 		num = 0;
-		spin_lock(&unix_table_locks[slot]);
 		spin_lock(&net->unx.table.locks[slot]);
 		sk_for_each(sk, &net->unx.table.buckets[slot]) {
 			if (num < s_num)
@@ -220,14 +219,12 @@  static int unix_diag_dump(struct sk_buff *skb, struct netlink_callback *cb)
 					 cb->nlh->nlmsg_seq,
 					 NLM_F_MULTI) < 0) {
 				spin_unlock(&net->unx.table.locks[slot]);
-				spin_unlock(&unix_table_locks[slot]);
 				goto done;
 			}
 next:
 			num++;
 		}
 		spin_unlock(&net->unx.table.locks[slot]);
-		spin_unlock(&unix_table_locks[slot]);
 	}
 done:
 	cb->args[0] = slot;
@@ -242,7 +239,6 @@  static struct sock *unix_lookup_by_ino(struct net *net, unsigned int ino)
 	int i;
 
 	for (i = 0; i < UNIX_HASH_SIZE; i++) {
-		spin_lock(&unix_table_locks[i]);
 		spin_lock(&net->unx.table.locks[i]);
 		sk_for_each(sk, &net->unx.table.buckets[i]) {
 			if (ino == sock_i_ino(sk)) {
@@ -253,7 +249,6 @@  static struct sock *unix_lookup_by_ino(struct net *net, unsigned int ino)
 			}
 		}
 		spin_unlock(&net->unx.table.locks[i]);
-		spin_unlock(&unix_table_locks[i]);
 	}
 	return NULL;
 }