From patchwork Thu Aug 3 01:28:00 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Andersson X-Patchwork-Id: 109280 Delivered-To: patch@linaro.org Received: by 10.140.101.6 with SMTP id t6csp142886qge; Wed, 2 Aug 2017 18:28:12 -0700 (PDT) X-Received: by 10.84.217.208 with SMTP id d16mr27226702plj.269.1501723692228; Wed, 02 Aug 2017 18:28:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1501723692; cv=none; d=google.com; s=arc-20160816; b=mfp9YJQQJhEEAqI5zpV2U3r/r+xbSkryUwThJPLfYB3aL4i5UH1B9fZXHAyaoTGrWi cd2UekqKcLN/VNCDao3LxNYAJXIkLonEi+ZngB4hIZ9kR3nz5juGu5XPT4YGhyaAu+oQ 8hYvGaqTftij4YWzQrjWNPrr4nXkOVDl/ST+cAX9LCOfpWPtOvYjMj9H4edP+V3MH2oS 2k+Wem7Yz68geznx2fOhefiHRsE1OIPcU2vGGVoVX1+W+9f+p4sMn17pSC6F8spHsKos HgITPMSvZ2MexpOhXMt04MVIrnkashd/mPwHqrJ6rjFyXQ4ywGbIG1uPzHADfOHNUHZS guiw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :dkim-signature:arc-authentication-results; bh=IRz4zgMxCmPaKVdlWedC2mYhorvHb1vymv1DXS1uGy8=; b=MMsFikSqL7LjLIXrDg3hw1/3uTLnUFIit2ezVuHgr8t4fajOtLWsqQZv/40hsv0j5e r0idQ+DWDi6jaLM0+NA51IM+Ea2Hc4OoX14Bxv9Z4DMhqx+SRDM2qfGo0L6e+vrDZj65 A+W6AQtwyHQmLRim+7QCS+wns8ZIvACFY9agJU3OgPkvQ2jipehOsj/xI0eDM/vItsYw GRWdk+ctKipXkHI1dv359ep/m0iH9+PhBxkmv+vyIM89+P/apxdnjPZg2m2+Qv1jPTj9 7dbPOuTFbi2F+A47nIH3ZKa0bBippg4LiukOIqP8LaNjWWohS/Y/eX1fUykn6ykOwUy+ d8iA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.b=eqYMq787; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id z124si17727067pfz.256.2017.08.02.18.28.11; Wed, 02 Aug 2017 18:28:12 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.b=eqYMq787; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752082AbdHCB2K (ORCPT + 25 others); Wed, 2 Aug 2017 21:28:10 -0400 Received: from mail-pf0-f174.google.com ([209.85.192.174]:36320 "EHLO mail-pf0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751941AbdHCB2E (ORCPT ); Wed, 2 Aug 2017 21:28:04 -0400 Received: by mail-pf0-f174.google.com with SMTP id z129so27746330pfb.3 for ; Wed, 02 Aug 2017 18:28:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=IRz4zgMxCmPaKVdlWedC2mYhorvHb1vymv1DXS1uGy8=; b=eqYMq787js2In8Txr5vKq/qIR8V5/c8aWibom9JhN0WPHL6PkGPMXkYfy2NtBmwyiq 2MrJkKqMM9Ov+hLBkYPPT0SzE1BgbQ4Bmj+/XgnpQlopL49GYJ+R2xM7UdqFD4GnWrOD CV5lOqAqZkLz+vxy4N8DbkguHfYM+PToilQwA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=IRz4zgMxCmPaKVdlWedC2mYhorvHb1vymv1DXS1uGy8=; b=IpQ8XZoAEczc/2g+vlW/miej8BqKh02n0T03/b7fH3JOeuaXD/ED11RZKHmoo8D380 TWgk2Z/8DAi7/CI7s17sxvLQoymGyakhlSrdZ9Jjq5KJ2paUWfcPfNLGvv/bdKtDd+MP 3Oe3utW2RCE7VdvVM2frBXhL5mvzUnM9U2VhUWjjQ/NurKWaB2AjweUlYi2KJ09UNib8 J/W+rTRTIZY1lxVg53IaoHzT4hmsb83IrDAyrtiMpG/Q4JejfQ9yBanDDKjKadtx35bH hy4JgAG0gt7uyy01j3IERUQS3z9pPZyBFFkUK2tCS641TTCxAZzU5/sz1B8NzF3dNJWe LpPA== X-Gm-Message-State: AIVw112I0L/W7f/w9nTbXkkgMSUM9WW5LQSHNCWVHDHHdr8XPwQi+FJ0 zOlB71/XRsQKX4V1AGzVFQ== X-Received: by 10.101.77.6 with SMTP id i6mr24062262pgt.181.1501723684029; Wed, 02 Aug 2017 18:28:04 -0700 (PDT) Received: from localhost.localdomain (ip68-111-217-79.sd.sd.cox.net. [68.111.217.79]) by smtp.gmail.com with ESMTPSA id w6sm18355433pgt.38.2017.08.02.18.28.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 02 Aug 2017 18:28:03 -0700 (PDT) From: Bjorn Andersson To: Eugene Krasnikov , Kalle Valo Cc: wcn36xx@lists.infradead.org, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH] wcn36xx: Introduce mutual exclusion of fw configuration Date: Wed, 2 Aug 2017 18:28:00 -0700 Message-Id: <20170803012800.15974-1-bjorn.andersson@linaro.org> X-Mailer: git-send-email 2.12.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org As the association status changes the driver needs to configure the hardware. This is done based on information in the "sta" acquired by ieee80211_find_sta(), which requires the caller to ensure that the "sta" is valid while its being used; generally by entering an rcu read section. But the operations acting on the "sta" has to communicate with the firmware and may therefor sleep, resulting in the following report: [ 31.418190] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:238 [ 31.425919] in_atomic(): 0, irqs_disabled(): 0, pid: 34, name: kworker/u8:1 [ 31.434609] CPU: 0 PID: 34 Comm: kworker/u8:1 Tainted: G W 4.12.0-rc4-next-20170607+ #993 [ 31.441002] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT) [ 31.450380] Workqueue: phy0 ieee80211_iface_work [ 31.457226] Call trace: [ 31.461830] [] dump_backtrace+0x0/0x260 [ 31.464004] [] show_stack+0x14/0x20 [ 31.469557] [] dump_stack+0x98/0xb8 [ 31.474592] [] ___might_sleep+0xf0/0x118 [ 31.479626] [] __might_sleep+0x50/0x88 [ 31.485010] [] mutex_lock+0x24/0x60 [ 31.490479] [] wcn36xx_smd_set_link_st+0x30/0x130 [ 31.495428] [] wcn36xx_bss_info_changed+0x148/0x448 [ 31.501504] [] ieee80211_bss_info_change_notify+0xbc/0x118 [ 31.508102] [] ieee80211_assoc_success+0x664/0x7f8 [ 31.515220] [] ieee80211_rx_mgmt_assoc_resp+0x144/0x2d8 [ 31.521555] [] ieee80211_sta_rx_queued_mgmt+0x190/0x698 [ 31.528239] [] ieee80211_iface_work+0x234/0x368 [ 31.535011] [] process_one_work+0x1cc/0x340 [ 31.541086] [] worker_thread+0x48/0x430 [ 31.546814] [] kthread+0x108/0x138 [ 31.552195] [] ret_from_fork+0x10/0x50 In order to ensure that the "sta" remains alive (and consistent) for the duration of bss_info_changed() mutual exclusion has to be ensured with sta_remove(). This is done by introducing a mutex to cover firmware configuration changes, which is made to also ensure mutual exclusion between other operations changing the state or configuration of the firmware. With this we can drop the rcu read lock. Cc: stable@vger.kernel.org Signed-off-by: Bjorn Andersson --- drivers/net/wireless/ath/wcn36xx/main.c | 52 ++++++++++++++++++++++++++++-- drivers/net/wireless/ath/wcn36xx/wcn36xx.h | 3 ++ 2 files changed, 53 insertions(+), 2 deletions(-) -- 2.12.0 diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c index d5e993dc9b23..6b6fa8e01f0e 100644 --- a/drivers/net/wireless/ath/wcn36xx/main.c +++ b/drivers/net/wireless/ath/wcn36xx/main.c @@ -372,6 +372,8 @@ static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed) wcn36xx_dbg(WCN36XX_DBG_MAC, "mac config changed 0x%08x\n", changed); + mutex_lock(&wcn->conf_mutex); + if (changed & IEEE80211_CONF_CHANGE_CHANNEL) { int ch = WCN36XX_HW_CHANNEL(wcn); wcn36xx_dbg(WCN36XX_DBG_MAC, "wcn36xx_config channel switch=%d\n", @@ -382,6 +384,8 @@ static int wcn36xx_config(struct ieee80211_hw *hw, u32 changed) } } + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -396,6 +400,8 @@ static void wcn36xx_configure_filter(struct ieee80211_hw *hw, wcn36xx_dbg(WCN36XX_DBG_MAC, "mac configure filter\n"); + mutex_lock(&wcn->conf_mutex); + *total &= FIF_ALLMULTI; fp = (void *)(unsigned long)multicast; @@ -408,6 +414,8 @@ static void wcn36xx_configure_filter(struct ieee80211_hw *hw, else if (NL80211_IFTYPE_STATION == vif->type && tmp->sta_assoc) wcn36xx_smd_set_mc_list(wcn, vif, fp); } + + mutex_unlock(&wcn->conf_mutex); kfree(fp); } @@ -471,6 +479,8 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd, key_conf->key, key_conf->keylen); + mutex_lock(&wcn->conf_mutex); + switch (key_conf->cipher) { case WLAN_CIPHER_SUITE_WEP40: vif_priv->encrypt_type = WCN36XX_HAL_ED_WEP40; @@ -565,6 +575,8 @@ static int wcn36xx_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd, } out: + mutex_unlock(&wcn->conf_mutex); + return ret; } @@ -725,6 +737,8 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw, wcn36xx_dbg(WCN36XX_DBG_MAC, "mac bss info changed vif %p changed 0x%08x\n", vif, changed); + mutex_lock(&wcn->conf_mutex); + if (changed & BSS_CHANGED_BEACON_INFO) { wcn36xx_dbg(WCN36XX_DBG_MAC, "mac bss changed dtim period %d\n", @@ -787,7 +801,13 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw, bss_conf->aid); vif_priv->sta_assoc = true; - rcu_read_lock(); + + /* + * Holding conf_mutex ensures mutal exclusion with + * wcn36xx_sta_remove() and as such ensures that sta + * won't be freed while we're operating on it. As such + * we do not need to hold the rcu_read_lock(). + */ sta = ieee80211_find_sta(vif, bss_conf->bssid); if (!sta) { wcn36xx_err("sta %pM is not found\n", @@ -811,7 +831,6 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw, * place where AID is available. */ wcn36xx_smd_config_sta(wcn, vif, sta); - rcu_read_unlock(); } else { wcn36xx_dbg(WCN36XX_DBG_MAC, "disassociated bss %pM vif %pM AID=%d\n", @@ -873,6 +892,9 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw, } } out: + + mutex_unlock(&wcn->conf_mutex); + return; } @@ -882,7 +904,10 @@ static int wcn36xx_set_rts_threshold(struct ieee80211_hw *hw, u32 value) struct wcn36xx *wcn = hw->priv; wcn36xx_dbg(WCN36XX_DBG_MAC, "mac set RTS threshold %d\n", value); + mutex_lock(&wcn->conf_mutex); wcn36xx_smd_update_cfg(wcn, WCN36XX_HAL_CFG_RTS_THRESHOLD, value); + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -893,8 +918,12 @@ static void wcn36xx_remove_interface(struct ieee80211_hw *hw, struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif); wcn36xx_dbg(WCN36XX_DBG_MAC, "mac remove interface vif %p\n", vif); + mutex_lock(&wcn->conf_mutex); + list_del(&vif_priv->list); wcn36xx_smd_delete_sta_self(wcn, vif->addr); + + mutex_unlock(&wcn->conf_mutex); } static int wcn36xx_add_interface(struct ieee80211_hw *hw, @@ -915,9 +944,13 @@ static int wcn36xx_add_interface(struct ieee80211_hw *hw, return -EOPNOTSUPP; } + mutex_lock(&wcn->conf_mutex); + list_add(&vif_priv->list, &wcn->vif_list); wcn36xx_smd_add_sta_self(wcn, vif); + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -930,6 +963,8 @@ static int wcn36xx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif, wcn36xx_dbg(WCN36XX_DBG_MAC, "mac sta add vif %p sta %pM\n", vif, sta->addr); + mutex_lock(&wcn->conf_mutex); + spin_lock_init(&sta_priv->ampdu_lock); sta_priv->vif = vif_priv; /* @@ -941,6 +976,9 @@ static int wcn36xx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif, sta_priv->aid = sta->aid; wcn36xx_smd_config_sta(wcn, vif, sta); } + + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -954,8 +992,13 @@ static int wcn36xx_sta_remove(struct ieee80211_hw *hw, wcn36xx_dbg(WCN36XX_DBG_MAC, "mac sta remove vif %p sta %pM index %d\n", vif, sta->addr, sta_priv->sta_index); + mutex_lock(&wcn->conf_mutex); + wcn36xx_smd_delete_sta(wcn, sta_priv->sta_index); sta_priv->vif = NULL; + + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -999,6 +1042,8 @@ static int wcn36xx_ampdu_action(struct ieee80211_hw *hw, wcn36xx_dbg(WCN36XX_DBG_MAC, "mac ampdu action action %d tid %d\n", action, tid); + mutex_lock(&wcn->conf_mutex); + switch (action) { case IEEE80211_AMPDU_RX_START: sta_priv->tid = tid; @@ -1038,6 +1083,8 @@ static int wcn36xx_ampdu_action(struct ieee80211_hw *hw, wcn36xx_err("Unknown AMPDU action\n"); } + mutex_unlock(&wcn->conf_mutex); + return 0; } @@ -1216,6 +1263,7 @@ static int wcn36xx_probe(struct platform_device *pdev) wcn = hw->priv; wcn->hw = hw; wcn->dev = &pdev->dev; + mutex_init(&wcn->conf_mutex); mutex_init(&wcn->hal_mutex); mutex_init(&wcn->scan_lock); diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h index b52b4da9a967..6aefba4c0cda 100644 --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h @@ -202,6 +202,9 @@ struct wcn36xx { struct qcom_smem_state *tx_rings_empty_state; unsigned tx_rings_empty_state_bit; + /* prevents concurrent FW reconfiguration */ + struct mutex conf_mutex; + /* * smd_buf must be protected with smd_mutex to garantee * that all messages are sent one after another