Re: net/core/dev.c:6917 napi_disable() error: uninitialized symbol 'new'.

[Jakub Kicinski]

On Thu, 11 Nov 2021 11:32:38 +0300 Dan Carpenter wrote:
> Jakub, could you change the netdev patchwork bot to include the Link:
> tag so I can look up the thread for this with b4?  The LKP robot sets
> the In-Reply-To header when it pulls the patch from a thread.  In this
> case, it's pulling it from a git tree but it could be modified to use
> the Link tag as well if it doesn't already.
> 
> I don't know how we would handle patches with multiple Link: tags,
> though.  I guess it's supposed to be in chronological order so the last
> lore.kernel.org tag is the correct one?

Hm, we'd need to coordinate this, because we have multiple people
applying patches from the netdevbpf pw instance and most add the 
Link tag locally.

Do you know if pw is capable of adding the Link so this would just 
be a switch for Konstantin to flip?

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   cb690f5238d71f543f4ce874aa59237cf53a877c
> commit: 719c571970109b0d0af24745d31b202affc9365f net: make napi_disable() symmetric with enable
> config: i386-randconfig-m021-20210928 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@xxxxxxxxx>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> 
> smatch warnings:
> net/core/dev.c:6917 napi_disable() error: uninitialized symbol 'new'.
> 
> vim +/new +6917 net/core/dev.c
> 
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6901  void napi_disable(struct napi_struct *n)
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6902  {
> 719c571970109b Jakub Kicinski 2021-09-24  6903  	unsigned long val, new;
> 719c571970109b Jakub Kicinski 2021-09-24  6904  
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6905  	might_sleep();
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6906  	set_bit(NAPI_STATE_DISABLE, &n->state);
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6907  
> 719c571970109b Jakub Kicinski 2021-09-24  6908  	do {
> 719c571970109b Jakub Kicinski 2021-09-24  6909  		val = READ_ONCE(n->state);
> 719c571970109b Jakub Kicinski 2021-09-24  6910  		if (val & (NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC)) {
> 719c571970109b Jakub Kicinski 2021-09-24  6911  			usleep_range(20, 200);
> 719c571970109b Jakub Kicinski 2021-09-24  6912  			continue;
> 
> Can we hit this continue on the first iteration through the loop?
> 
> 719c571970109b Jakub Kicinski 2021-09-24  6913  		}
> 719c571970109b Jakub Kicinski 2021-09-24  6914  
> 719c571970109b Jakub Kicinski 2021-09-24  6915  		new = val | NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC;
> 719c571970109b Jakub Kicinski 2021-09-24  6916  		new &= ~(NAPIF_STATE_THREADED | NAPIF_STATE_PREFER_BUSY_POLL);
> 719c571970109b Jakub Kicinski 2021-09-24 @6917  	} while (cmpxchg(&n->state, val, new) != val);
>                                                                                          ^^^
> Warning.
> 
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6918  
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6919  	hrtimer_cancel(&n->timer);
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6920  
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6921  	clear_bit(NAPI_STATE_DISABLE, &n->state);
> 3b47d30396bae4 Eric Dumazet   2014-11-06  6922  }

Indeed, fixed yesterday by 0315a075f134 ("net: fix premature exit from
NAPI state polling in napi_disable()"). Thanks!