wifi: iwlwifi: mvm: Fix race in scan completion

The move of the scan complete notification handling to the wiphy worker
introduced a race between scan complete notification and scan abort:

- The wiphy lock is held, e.g., for rfkill handling etc.
- Scan complete notification is received but not handled yet.
- Scan abort is triggered, and scan abort is sent to the FW. Once the
  scan abort command is sent successfully, the flow synchronously waits
  for the scan complete notification. However, as the scan complete
  notification was already received but not processed yet, this hangs for
  a second and continues leaving the scan status in an inconsistent
  state.
- Once scan complete handling is started (when the wiphy lock is not held)
  since the scan status is not an inconsistent state, a warning is issued
  and the scan complete notification is not handled.

To fix this issue, switch back the scan complete notification to be
asynchronously handling, and only move the link selection logic to
a worker (which was the original reason for the move to use wiphy lock).

While at it, refactor some prints to improve debug data.

Fixes: 07bf5297d392 ("wifi: iwlwifi: mvm: Implement new link selection algorithm")
Signed-off-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com>
Link: https://msgid.link/20240506095953.1f484a86324b.I63ed445a47f144546948c74ae6df85587fdb4ce3@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index 57a0d8af9b28ab8b58aa4604bca5ef7d39f068c8..fb49deda3346cb0b76ccc20511b7f08c0bbdc2a3 100644 (file)
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -1350,6 +1350,7 @@ void iwl_mvm_mac_stop(struct ieee80211_hw *hw)
        iwl_mvm_scan_stop(mvm, IWL_MVM_SCAN_INT_MLO, false);
        mutex_unlock(&mvm->mutex);
 
+       wiphy_work_cancel(mvm->hw->wiphy, &mvm->trig_link_selection_wk);
        wiphy_work_flush(mvm->hw->wiphy, &mvm->async_handlers_wiphy_wk);
        flush_work(&mvm->async_handlers_wk);
        flush_work(&mvm->add_stream_wk);

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
index cb4088149d85242b0b6e394a3c66f6a78d68a2f5..b292276de4ae524e2357b1ccb2d2ba0cbcd27aab 100644 (file)
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mvm.h
@@ -937,6 +937,8 @@ struct iwl_mvm {
        /* For async rx handlers that require the wiphy lock */
        struct wiphy_work async_handlers_wiphy_wk;
 
+       struct wiphy_work trig_link_selection_wk;
+
        struct work_struct roc_done_wk;
 
        unsigned long init_status;

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
index 155a44e8ab0727708ffd27ef7931643a1537fd6c..b27a032079387cfa76ddc93d6ca60268d2e25796 100644 (file)
--- a/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/ops.c
@@ -383,7 +383,7 @@ static const struct iwl_rx_handlers iwl_mvm_rx_handlers[] = {
                           iwl_mvm_rx_scan_match_found,
                           RX_HANDLER_SYNC),
        RX_HANDLER(SCAN_COMPLETE_UMAC, iwl_mvm_rx_umac_scan_complete_notif,
-                  RX_HANDLER_ASYNC_LOCKED_WIPHY,
+                  RX_HANDLER_ASYNC_LOCKED,
                   struct iwl_umac_scan_complete),
        RX_HANDLER(SCAN_ITERATION_COMPLETE_UMAC,
                   iwl_mvm_rx_umac_scan_iter_complete_notif, RX_HANDLER_SYNC,
@@ -1171,6 +1171,27 @@ static const struct iwl_mei_ops mei_ops = {
        .nic_stolen = iwl_mvm_mei_nic_stolen,
 };
 
+static void iwl_mvm_find_link_selection_vif(void *_data, u8 *mac,
+                                           struct ieee80211_vif *vif)
+{
+       struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
+
+       if (ieee80211_vif_is_mld(vif) && mvmvif->authorized)
+               iwl_mvm_select_links(mvmvif->mvm, vif);
+}
+
+static void iwl_mvm_trig_link_selection(struct wiphy *wiphy,
+                                       struct wiphy_work *wk)
+{
+       struct iwl_mvm *mvm =
+               container_of(wk, struct iwl_mvm, trig_link_selection_wk);
+
+       ieee80211_iterate_active_interfaces(mvm->hw,
+                                           IEEE80211_IFACE_ITER_NORMAL,
+                                           iwl_mvm_find_link_selection_vif,
+                                           NULL);
+}
+
 static struct iwl_op_mode *
 iwl_op_mode_mvm_start(struct iwl_trans *trans, const struct iwl_cfg *cfg,
                      const struct iwl_fw *fw, struct dentry *dbgfs_dir)
@@ -1302,6 +1323,10 @@ iwl_op_mode_mvm_start(struct iwl_trans *trans, const struct iwl_cfg *cfg,
 
        wiphy_work_init(&mvm->async_handlers_wiphy_wk,
                        iwl_mvm_async_handlers_wiphy_wk);
+
+       wiphy_work_init(&mvm->trig_link_selection_wk,
+                       iwl_mvm_trig_link_selection);
+
        init_waitqueue_head(&mvm->rx_sync_waitq);
 
        mvm->queue_sync_state = 0;

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/scan.c b/drivers/net/wireless/intel/iwlwifi/mvm/scan.c
index 433280b3c03ebdd3e11eafd6b2bd309b75ca808e..49ec515b5bad63177812be7a677b014f36f0e3cc 100644 (file)
--- a/drivers/net/wireless/intel/iwlwifi/mvm/scan.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/scan.c
@@ -3178,23 +3178,6 @@ int iwl_mvm_sched_scan_start(struct iwl_mvm *mvm,
        return ret;
 }
 
-static void iwl_mvm_find_link_selection_vif(void *_data, u8 *mac,
-                                           struct ieee80211_vif *vif)
-{
-       struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
-
-       if (ieee80211_vif_is_mld(vif) && mvmvif->authorized)
-               iwl_mvm_select_links(mvmvif->mvm, vif);
-}
-
-static void iwl_mvm_post_scan_link_selection(struct iwl_mvm *mvm)
-{
-       ieee80211_iterate_active_interfaces(mvm->hw,
-                                           IEEE80211_IFACE_ITER_NORMAL,
-                                           iwl_mvm_find_link_selection_vif,
-                                           NULL);
-}
-
 void iwl_mvm_rx_umac_scan_complete_notif(struct iwl_mvm *mvm,
                                         struct iwl_rx_cmd_buffer *rxb)
 {
@@ -3206,6 +3189,21 @@ void iwl_mvm_rx_umac_scan_complete_notif(struct iwl_mvm *mvm,
 
        mvm->mei_scan_filter.is_mei_limited_scan = false;
 
+       IWL_DEBUG_SCAN(mvm,
+                      "Scan completed: uid=%u type=%u, status=%s, EBS=%s\n",
+                      uid, mvm->scan_uid_status[uid],
+                      notif->status == IWL_SCAN_OFFLOAD_COMPLETED ?
+                               "completed" : "aborted",
+                      iwl_mvm_ebs_status_str(notif->ebs_status));
+
+       IWL_DEBUG_SCAN(mvm, "Scan completed: scan_status=0x%x\n",
+                      mvm->scan_status);
+
+       IWL_DEBUG_SCAN(mvm,
+                      "Scan completed: line=%u, iter=%u, elapsed time=%u\n",
+                      notif->last_schedule, notif->last_iter,
+                      __le32_to_cpu(notif->time_from_last_iter));
+
        if (WARN_ON(!(mvm->scan_uid_status[uid] & mvm->scan_status)))
                return;
 
@@ -3244,16 +3242,9 @@ void iwl_mvm_rx_umac_scan_complete_notif(struct iwl_mvm *mvm,
        }
 
        mvm->scan_status &= ~mvm->scan_uid_status[uid];
-       IWL_DEBUG_SCAN(mvm,
-                      "Scan completed, uid %u type %u, status %s, EBS status %s\n",
-                      uid, mvm->scan_uid_status[uid],
-                      notif->status == IWL_SCAN_OFFLOAD_COMPLETED ?
-                               "completed" : "aborted",
-                      iwl_mvm_ebs_status_str(notif->ebs_status));
-       IWL_DEBUG_SCAN(mvm,
-                      "Last line %d, Last iteration %d, Time from last iteration %d\n",
-                      notif->last_schedule, notif->last_iter,
-                      __le32_to_cpu(notif->time_from_last_iter));
+
+       IWL_DEBUG_SCAN(mvm, "Scan completed: after update: scan_status=0x%x\n",
+                      mvm->scan_status);
 
        if (notif->ebs_status != IWL_SCAN_EBS_SUCCESS &&
            notif->ebs_status != IWL_SCAN_EBS_INACTIVE)
@@ -3262,7 +3253,7 @@ void iwl_mvm_rx_umac_scan_complete_notif(struct iwl_mvm *mvm,
        mvm->scan_uid_status[uid] = 0;
 
        if (select_links)
-               iwl_mvm_post_scan_link_selection(mvm);
+               wiphy_work_queue(mvm->hw->wiphy, &mvm->trig_link_selection_wk);
 }
 
 void iwl_mvm_rx_umac_scan_iter_complete_notif(struct iwl_mvm *mvm,
@@ -3487,6 +3478,10 @@ int iwl_mvm_scan_stop(struct iwl_mvm *mvm, int type, bool notify)
 {
        int ret;
 
+       IWL_DEBUG_SCAN(mvm,
+                      "Request to stop scan: type=0x%x, status=0x%x\n",
+                      type, mvm->scan_status);
+
        if (!(mvm->scan_status & type))
                return 0;
 
@@ -3498,6 +3493,9 @@ int iwl_mvm_scan_stop(struct iwl_mvm *mvm, int type, bool notify)
        ret = iwl_mvm_scan_stop_wait(mvm, type);
        if (!ret)
                mvm->scan_status |= type << IWL_MVM_SCAN_STOPPING_SHIFT;
+       else
+               IWL_DEBUG_SCAN(mvm, "Failed to stop scan\n");
+
 out:
        /* Clear the scan status so the next scan requests will
         * succeed and mark the scan as stopping, so that the Rx