diff mbox series

[net-next,2/4] inetpeer: remove create argument of inet_getpeer()

Message ID 20241213130212.1783302-3-edumazet@google.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series inetpeer: reduce false sharing and atomic operations | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 10 this patch: 11
netdev/build_tools success Errors and warnings before: 0 (+23) this patch: 0 (+23)
netdev/cc_maintainers success CCed 6 of 6 maintainers
netdev/build_clang fail Errors and warnings before: 16 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
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: 1819 this patch: 1820
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 52 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Eric Dumazet Dec. 13, 2024, 1:02 p.m. UTC 
All callers of inet_getpeer() want to create an inetpeer.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/inetpeer.h | 7 +++----
 net/ipv4/inetpeer.c    | 9 ++-------
 2 files changed, 5 insertions(+), 11 deletions(-)

Comments

Simon Horman Dec. 13, 2024, 8:16 p.m. UTC | #1 
On Fri, Dec 13, 2024 at 01:02:10PM +0000, Eric Dumazet wrote:
> All callers of inet_getpeer() want to create an inetpeer.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>

...

> diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c

...

> @@ -189,10 +188,6 @@ struct inet_peer *inet_getpeer(struct inet_peer_base *base,
>  	if (p)
>  		return p;
>  
> -	/* If no writer did a change during our lookup, we can return early. */
> -	if (!create && !invalidated)
> -		return NULL;
> -

Hi Eric,

With this change invalidated is set but otherwise unused in this function,
so it can probably be removed.

Flagged by W=1 builds.

>  	/* retry an exact lookup, taking the lock before.
>  	 * At least, nodes should be hot in our cache.
>  	 */
Eric Dumazet Dec. 13, 2024, 8:47 p.m. UTC | #2 
On Fri, Dec 13, 2024 at 9:16 PM Simon Horman <horms@kernel.org> wrote:

> Hi Eric,
>
> With this change invalidated is set but otherwise unused in this function,
> so it can probably be removed.
>
> Flagged by W=1 builds.

Ah, right, I will fix this in V2.

Thanks Simon.
kernel test robot Dec. 14, 2024, 2:34 p.m. UTC | #3 
Hi Eric,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Eric-Dumazet/inetpeer-remove-create-argument-of-inet_getpeer_v-46/20241213-210500
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241213130212.1783302-3-edumazet%40google.com
patch subject: [PATCH net-next 2/4] inetpeer: remove create argument of inet_getpeer()
config: i386-buildonly-randconfig-003-20241214 (https://download.01.org/0day-ci/archive/20241214/202412142229.7lFHEOun-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241214/202412142229.7lFHEOun-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412142229.7lFHEOun-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from net/ipv4/inetpeer.c:19:
   In file included from include/linux/mm.h:2223:
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
>> net/ipv4/inetpeer.c:177:6: warning: variable 'invalidated' set but not used [-Wunused-but-set-variable]
     177 |         int invalidated;
         |             ^
   2 warnings generated.


vim +/invalidated +177 net/ipv4/inetpeer.c

^1da177e4c3f41 Linus Torvalds   2005-04-16  170  
c0efc887dcadbd David S. Miller  2012-06-09  171  struct inet_peer *inet_getpeer(struct inet_peer_base *base,
4a6cb3d13bf1f8 Eric Dumazet     2024-12-13  172  			       const struct inetpeer_addr *daddr)
^1da177e4c3f41 Linus Torvalds   2005-04-16  173  {
b145425f269a17 Eric Dumazet     2017-07-17  174  	struct inet_peer *p, *gc_stack[PEER_MAX_GC];
b145425f269a17 Eric Dumazet     2017-07-17  175  	struct rb_node **pp, *parent;
b145425f269a17 Eric Dumazet     2017-07-17  176  	unsigned int gc_cnt, seq;
b145425f269a17 Eric Dumazet     2017-07-17 @177  	int invalidated;
^1da177e4c3f41 Linus Torvalds   2005-04-16  178  
4b9d9be839fdb7 Eric Dumazet     2011-06-08  179  	/* Attempt a lockless lookup first.
aa1039e73cc2cf Eric Dumazet     2010-06-15  180  	 * Because of a concurrent writer, we might not find an existing entry.
aa1039e73cc2cf Eric Dumazet     2010-06-15  181  	 */
7b46ac4e77f322 David S. Miller  2011-03-08  182  	rcu_read_lock();
b145425f269a17 Eric Dumazet     2017-07-17  183  	seq = read_seqbegin(&base->lock);
b145425f269a17 Eric Dumazet     2017-07-17  184  	p = lookup(daddr, base, seq, NULL, &gc_cnt, &parent, &pp);
b145425f269a17 Eric Dumazet     2017-07-17  185  	invalidated = read_seqretry(&base->lock, seq);
7b46ac4e77f322 David S. Miller  2011-03-08  186  	rcu_read_unlock();
^1da177e4c3f41 Linus Torvalds   2005-04-16  187  
4b9d9be839fdb7 Eric Dumazet     2011-06-08  188  	if (p)
aa1039e73cc2cf Eric Dumazet     2010-06-15  189  		return p;
aa1039e73cc2cf Eric Dumazet     2010-06-15  190  
aa1039e73cc2cf Eric Dumazet     2010-06-15  191  	/* retry an exact lookup, taking the lock before.
aa1039e73cc2cf Eric Dumazet     2010-06-15  192  	 * At least, nodes should be hot in our cache.
aa1039e73cc2cf Eric Dumazet     2010-06-15  193  	 */
b145425f269a17 Eric Dumazet     2017-07-17  194  	parent = NULL;
65e8354ec13a45 Eric Dumazet     2011-03-04  195  	write_seqlock_bh(&base->lock);
b145425f269a17 Eric Dumazet     2017-07-17  196  
b145425f269a17 Eric Dumazet     2017-07-17  197  	gc_cnt = 0;
b145425f269a17 Eric Dumazet     2017-07-17  198  	p = lookup(daddr, base, seq, gc_stack, &gc_cnt, &parent, &pp);
4a6cb3d13bf1f8 Eric Dumazet     2024-12-13  199  	if (!p) {
b145425f269a17 Eric Dumazet     2017-07-17  200  		p = kmem_cache_alloc(peer_cachep, GFP_ATOMIC);
aa1039e73cc2cf Eric Dumazet     2010-06-15  201  		if (p) {
b534ecf1cd26f0 David S. Miller  2010-11-30  202  			p->daddr = *daddr;
b6a37e5e25414d Eric Dumazet     2018-04-09  203  			p->dtime = (__u32)jiffies;
1cc9a98b59ba92 Reshetova, Elena 2017-06-30  204  			refcount_set(&p->refcnt, 2);
aa1039e73cc2cf Eric Dumazet     2010-06-15  205  			atomic_set(&p->rid, 0);
144001bddcb4db David S. Miller  2011-01-27  206  			p->metrics[RTAX_LOCK-1] = INETPEER_METRICS_NEW;
92d8682926342d David S. Miller  2011-02-04  207  			p->rate_tokens = 0;
c09551c6ff7fe1 Lorenzo Bianconi 2019-02-06  208  			p->n_redirects = 0;
bc9259a8bae9e8 Nicolas Dichtel  2012-09-27  209  			/* 60*HZ is arbitrary, but chosen enough high so that the first
bc9259a8bae9e8 Nicolas Dichtel  2012-09-27  210  			 * calculation of tokens is at its maximum.
bc9259a8bae9e8 Nicolas Dichtel  2012-09-27  211  			 */
bc9259a8bae9e8 Nicolas Dichtel  2012-09-27  212  			p->rate_last = jiffies - 60*HZ;
^1da177e4c3f41 Linus Torvalds   2005-04-16  213  
b145425f269a17 Eric Dumazet     2017-07-17  214  			rb_link_node(&p->rb_node, parent, pp);
b145425f269a17 Eric Dumazet     2017-07-17  215  			rb_insert_color(&p->rb_node, &base->rb_root);
98158f5a853caf David S. Miller  2010-11-30  216  			base->total++;
aa1039e73cc2cf Eric Dumazet     2010-06-15  217  		}
b145425f269a17 Eric Dumazet     2017-07-17  218  	}
b145425f269a17 Eric Dumazet     2017-07-17  219  	if (gc_cnt)
b145425f269a17 Eric Dumazet     2017-07-17  220  		inet_peer_gc(base, gc_stack, gc_cnt);
65e8354ec13a45 Eric Dumazet     2011-03-04  221  	write_sequnlock_bh(&base->lock);
^1da177e4c3f41 Linus Torvalds   2005-04-16  222  
^1da177e4c3f41 Linus Torvalds   2005-04-16  223  	return p;
^1da177e4c3f41 Linus Torvalds   2005-04-16  224  }
b3419363808f24 David S. Miller  2010-11-30  225  EXPORT_SYMBOL_GPL(inet_getpeer);
98158f5a853caf David S. Miller  2010-11-30  226
diff mbox series

Patch

diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index 6f51f81d6cb19c623e9b347dbdbbd8d849848f6e..f475757daafba998a10c815d0178c98d2bf1ae43 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -96,8 +96,7 @@  static inline struct in6_addr *inetpeer_get_addr_v6(struct inetpeer_addr *iaddr)
 
 /* can be called with or without local BH being disabled */
 struct inet_peer *inet_getpeer(struct inet_peer_base *base,
-			       const struct inetpeer_addr *daddr,
-			       int create);
+			       const struct inetpeer_addr *daddr);
 
 static inline struct inet_peer *inet_getpeer_v4(struct inet_peer_base *base,
 						__be32 v4daddr,
@@ -108,7 +107,7 @@  static inline struct inet_peer *inet_getpeer_v4(struct inet_peer_base *base,
 	daddr.a4.addr = v4daddr;
 	daddr.a4.vif = vif;
 	daddr.family = AF_INET;
-	return inet_getpeer(base, &daddr, 1);
+	return inet_getpeer(base, &daddr);
 }
 
 static inline struct inet_peer *inet_getpeer_v6(struct inet_peer_base *base,
@@ -118,7 +117,7 @@  static inline struct inet_peer *inet_getpeer_v6(struct inet_peer_base *base,
 
 	daddr.a6 = *v6daddr;
 	daddr.family = AF_INET6;
-	return inet_getpeer(base, &daddr, 1);
+	return inet_getpeer(base, &daddr);
 }
 
 static inline int inetpeer_addr_cmp(const struct inetpeer_addr *a,
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 5ab56f4cb529769d4edb07261c08d61ff96f0c0f..58d2805b046d00cd509e2d2343abfb8eacfbdde7 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -169,8 +169,7 @@  static void inet_peer_gc(struct inet_peer_base *base,
 }
 
 struct inet_peer *inet_getpeer(struct inet_peer_base *base,
-			       const struct inetpeer_addr *daddr,
-			       int create)
+			       const struct inetpeer_addr *daddr)
 {
 	struct inet_peer *p, *gc_stack[PEER_MAX_GC];
 	struct rb_node **pp, *parent;
@@ -189,10 +188,6 @@  struct inet_peer *inet_getpeer(struct inet_peer_base *base,
 	if (p)
 		return p;
 
-	/* If no writer did a change during our lookup, we can return early. */
-	if (!create && !invalidated)
-		return NULL;
-
 	/* retry an exact lookup, taking the lock before.
 	 * At least, nodes should be hot in our cache.
 	 */
@@ -201,7 +196,7 @@  struct inet_peer *inet_getpeer(struct inet_peer_base *base,
 
 	gc_cnt = 0;
 	p = lookup(daddr, base, seq, gc_stack, &gc_cnt, &parent, &pp);
-	if (!p && create) {
+	if (!p) {
 		p = kmem_cache_alloc(peer_cachep, GFP_ATOMIC);
 		if (p) {
 			p->daddr = *daddr;