re: wil6210: ADDBA/DELBA flows

re: wil6210: ADDBA/DELBA flows

[Dan Carpenter]

Hello Vladimir Kondratiev,

The patch 3277213feb1b: "wil6210: ADDBA/DELBA flows" from Dec 23,
2014, leads to the following static checker warning:

	drivers/net/wireless/ath/wil6210/rx_reorder.c:205 wil_tid_ampdu_rx_alloc()
	error: scheduling with locks held: 'spin_lock:tid_rx_lock'

drivers/net/wireless/ath/wil6210/rx_reorder.c
   202  struct wil_tid_ampdu_rx *wil_tid_ampdu_rx_alloc(struct wil6210_priv *wil,
   203                                                  int size, u16 ssn)
   204  {
   205          struct wil_tid_ampdu_rx *r = kzalloc(sizeof(*r), GFP_KERNEL);
                                                                 ^^^^^^^^^^
   206  
   207          if (!r)
   208                  return NULL;
   209  
   210          r->reorder_buf =
   211                  kcalloc(size, sizeof(struct sk_buff *), GFP_KERNEL);
                                                                ^^^^^^^^^^^
   212          r->reorder_time =
   213                  kcalloc(size, sizeof(unsigned long), GFP_KERNEL);
                                                             ^^^^^^^^^^^
   214          if (!r->reorder_buf || !r->reorder_time) {
   215                  kfree(r->reorder_buf);
   216                  kfree(r->reorder_time);
   217                  kfree(r);
   218                  return NULL;
   219          }
   220  

[ snip ]

   331          spin_lock_bh(&sta->tid_rx_lock);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
spin lock held.

   332  
   333          wil_tid_ampdu_rx_free(wil, sta->tid_rx[tid]);
   334          sta->tid_rx[tid] = wil_tid_ampdu_rx_alloc(wil, agg_wsize, ssn);
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
function called with the lock held.

   335  
   336          spin_unlock_bh(&sta->tid_rx_lock);

This looks like it would be pretty easy to re-write so the allocation
was done before we take the lock if someone knew exactly what the lock
was protecting.

regards,
dan carpenter

Re: wil6210: ADDBA/DELBA flows

[Vladimir Kondratiev]

On Monday, January 19, 2015 05:53:32 PM Dan Carpenter wrote:
> The patch 3277213feb1b: "wil6210: ADDBA/DELBA flows" from Dec 23,
> 2014, leads to the following static checker warning:
> 
>         drivers/net/wireless/ath/wil6210/rx_reorder.c:205 wil_tid_ampdu_rx_alloc()
>         error: scheduling with locks held: 'spin_lock:tid_rx_lock'
> 

Dan, thanks for finding this. Will submit patch shortly

Thanks, Vladimir