[bug report] iwlwifi: mvm: kill INACTIVE queue state

From: Dan Carpenter <dan.carpenter@oracle.com>
To: johannes.berg@intel.com
Cc: linux-wireless@vger.kernel.org
Subject: [bug report] iwlwifi: mvm: kill INACTIVE queue state
Date: Wed, 17 Oct 2018 10:55:34 +0300	[thread overview]
Message-ID: <20181017075534.anxr76wuf2rg4xxe@kili.mountain> (raw)

Hello Johannes Berg,

The patch 724fe7710ac5: "iwlwifi: mvm: kill INACTIVE queue state"
from Jul 4, 2018, leads to the following static checker warning:

	drivers/net/wireless/intel/iwlwifi/mvm/sta.c:1243 iwl_mvm_inactivity_check()
	warn: 'iwl_mvm_remove_inactive_tids(mvm, mvmsta, i, inactive_tid_bitmap, &unshare_queues, &changetid_queues)' is unsigned

drivers/net/wireless/intel/iwlwifi/mvm/sta.c
  1223                  /*
  1224                   * If the STA doesn't exist anymore, it isn't an error. It could
  1225                   * be that it was removed since getting the queues, and in this
  1226                   * case it should've inactivated its queues anyway.
  1227                   */
  1228                  if (IS_ERR_OR_NULL(sta))
  1229                          continue;
  1230  
  1231                  mvmsta = iwl_mvm_sta_from_mac80211(sta);
  1232  
  1233                  /* this isn't so nice, but works OK due to the way we loop */
  1234                  spin_unlock(&mvm->queue_info_lock);
  1235  
  1236                  /* and we need this locking order */
  1237                  spin_lock(&mvmsta->lock);
  1238                  spin_lock(&mvm->queue_info_lock);
  1239                  ret = iwl_mvm_remove_inactive_tids(mvm, mvmsta, i,
  1240                                                     inactive_tid_bitmap,
  1241                                                     &unshare_queues,
  1242                                                     &changetid_queues);
  1243                  if (ret >= 0 && free_queue < 0)
                            ^^^^^^^^

The iwl_mvm_remove_inactive_tids() returns a bool so it doesn't make
sense to test for >= 0.  Probably we should test for ret == true?

  1244                          free_queue = ret;
  1245                  /* only unlock sta lock - we still need the queue info lock */
  1246                  spin_unlock(&mvmsta->lock);
  1247          }

regards,
dan carpenter