From: Douglas Anderson <[email protected]>
There's absolutely no reason to check to see if a list is empty
before iterating through it. It's just like writing code like
this:
if (count != 0) {
for (i = 0; i < count; i++) {
...
}
}
The loop will already be avoided if "count == 0" so there was no
reason to check.
Signed-off-by: Douglas Anderson <[email protected]>
Signed-off-by: Ganapathi Bhat <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/11n.c | 9 ---------
drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c | 6 ------
drivers/net/wireless/marvell/mwifiex/init.c | 4 ----
drivers/net/wireless/marvell/mwifiex/tdls.c | 7 -------
4 files changed, 26 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/11n.c b/drivers/net/wireless/marvell/mwifiex/11n.c
index 7252069..8772e39 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n.c
@@ -658,12 +658,6 @@ void mwifiex_11n_delba(struct mwifiex_private *priv, int tid)
unsigned long flags;
spin_lock_irqsave(&priv->rx_reorder_tbl_lock, flags);
- if (list_empty(&priv->rx_reorder_tbl_ptr)) {
- dev_dbg(priv->adapter->dev,
- "mwifiex_11n_delba: rx_reorder_tbl_ptr empty\n");
- goto exit;
- }
-
list_for_each_entry(rx_reor_tbl_ptr, &priv->rx_reorder_tbl_ptr, list) {
if (rx_reor_tbl_ptr->tid == tid) {
dev_dbg(priv->adapter->dev,
@@ -854,9 +848,6 @@ u8 mwifiex_get_sec_chan_offset(int chan)
struct mwifiex_adapter *adapter = priv->adapter;
struct mwifiex_tx_ba_stream_tbl *tx_ba_stream_tbl_ptr;
- if (list_empty(&priv->tx_ba_stream_tbl_ptr))
- return;
-
list_for_each_entry(tx_ba_stream_tbl_ptr,
&priv->tx_ba_stream_tbl_ptr, list) {
if (tx_ba_stream_tbl_ptr->ba_status == BA_SETUP_COMPLETE) {
diff --git a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
index 274dd5a..d87df2d 100644
--- a/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
+++ b/drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c
@@ -835,12 +835,6 @@ void mwifiex_update_rxreor_flags(struct mwifiex_adapter *adapter, u8 flags)
continue;
spin_lock_irqsave(&priv->rx_reorder_tbl_lock, lock_flags);
- if (list_empty(&priv->rx_reorder_tbl_ptr)) {
- spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock,
- lock_flags);
- continue;
- }
-
list_for_each_entry(tbl, &priv->rx_reorder_tbl_ptr, list)
tbl->flags = flags;
spin_unlock_irqrestore(&priv->rx_reorder_tbl_lock, lock_flags);
diff --git a/drivers/net/wireless/marvell/mwifiex/init.c b/drivers/net/wireless/marvell/mwifiex/init.c
index e11919d..1176706 100644
--- a/drivers/net/wireless/marvell/mwifiex/init.c
+++ b/drivers/net/wireless/marvell/mwifiex/init.c
@@ -579,10 +579,6 @@ static void mwifiex_delete_bss_prio_tbl(struct mwifiex_private *priv)
{
spin_lock_irqsave(lock, flags);
- if (list_empty(head)) {
- spin_unlock_irqrestore(lock, flags);
- continue;
- }
list_for_each_entry_safe(bssprio_node, tmp_node, head,
list) {
if (bssprio_node->priv == priv) {
diff --git a/drivers/net/wireless/marvell/mwifiex/tdls.c b/drivers/net/wireless/marvell/mwifiex/tdls.c
index e76af286..9fe0bae 100644
--- a/drivers/net/wireless/marvell/mwifiex/tdls.c
+++ b/drivers/net/wireless/marvell/mwifiex/tdls.c
@@ -1413,13 +1413,6 @@ void mwifiex_check_auto_tdls(unsigned long context)
priv->check_tdls_tx = false;
- if (list_empty(&priv->auto_tdls_list)) {
- mod_timer(&priv->auto_tdls_timer,
- jiffies +
- msecs_to_jiffies(MWIFIEX_TIMER_10S));
- return;
- }
-
spin_lock_irqsave(&priv->auto_tdls_lock, flags);
list_for_each_entry(tdls_peer, &priv->auto_tdls_list, list) {
if ((jiffies - tdls_peer->rssi_jiffies) >
--
1.9.1
Ganapathi Bhat <[email protected]> wrote:
> From: Douglas Anderson <[email protected]>
>
> There's absolutely no reason to check to see if a list is empty
> before iterating through it. It's just like writing code like
> this:
>
> if (count != 0) {
> for (i = 0; i < count; i++) {
> ...
> }
> }
>
> The loop will already be avoided if "count == 0" so there was no
> reason to check.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> Signed-off-by: Ganapathi Bhat <[email protected]>
2 patches applied to wireless-drivers-next.git, thanks.
40351051d022 mwifiex: kill useless list_empty checks
f0f7c2275fb9 mwifiex: minor cleanups w/ sta_list_spinlock in cfg80211.c
--
https://patchwork.kernel.org/patch/9983061/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
From: Douglas Anderson <[email protected]>
The sta_list_spinlock looks to be used to control locking of the
list. Specifically when someone has the lock they may be allowed
to modify or delete elements of the list.
That implies that we shouldn't access the fields of the elements
returned by mwifiex_get_sta_entry() after we've released the
spinlock. Let's make some small changes so this is true.
It's unlikely that this matters since it looks to be just error
handling, but it's nice to be clean.
Signed-off-by: Douglas Anderson <[email protected]>
Signed-off-by: Ganapathi Bhat <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 14 +++++++++-----
drivers/net/wireless/marvell/mwifiex/sta_event.c | 6 ++----
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index cc7d777..3638b613 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3794,9 +3794,8 @@ static int mwifiex_cfg80211_set_coalesce(struct wiphy *wiphy,
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, addr);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (!sta_ptr) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "%s: Invalid TDLS peer %pM\n",
__func__, addr);
return -ENOENT;
@@ -3804,15 +3803,18 @@ static int mwifiex_cfg80211_set_coalesce(struct wiphy *wiphy,
if (!(sta_ptr->tdls_cap.extcap.ext_capab[3] &
WLAN_EXT_CAPA4_TDLS_CHAN_SWITCH)) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "%pM do not support tdls cs\n", addr);
return -ENOENT;
}
if (sta_ptr->tdls_status == TDLS_CHAN_SWITCHING ||
sta_ptr->tdls_status == TDLS_IN_OFF_CHAN) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "channel switch is running, abort request\n");
return -EALREADY;
}
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
chan = chandef->chan->hw_value;
second_chan_offset = mwifiex_get_sec_chan_offset(chan);
@@ -3833,18 +3835,20 @@ static int mwifiex_cfg80211_set_coalesce(struct wiphy *wiphy,
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, addr);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (!sta_ptr) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "%s: Invalid TDLS peer %pM\n",
__func__, addr);
} else if (!(sta_ptr->tdls_status == TDLS_CHAN_SWITCHING ||
sta_ptr->tdls_status == TDLS_IN_BASE_CHAN ||
sta_ptr->tdls_status == TDLS_IN_OFF_CHAN)) {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
wiphy_err(wiphy, "tdls chan switch not initialize by %pM\n",
addr);
- } else
+ } else {
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
mwifiex_stop_tdls_cs(priv, addr);
+ }
}
static int
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_event.c b/drivers/net/wireless/marvell/mwifiex/sta_event.c
index 839df8a..d8db412 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_event.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_event.c
@@ -359,13 +359,12 @@ static void mwifiex_process_uap_tx_pause(struct mwifiex_private *priv,
} else {
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, tp->peermac);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (sta_ptr && sta_ptr->tx_pause != tp->tx_pause) {
sta_ptr->tx_pause = tp->tx_pause;
mwifiex_update_ralist_tx_pause(priv, tp->peermac,
tp->tx_pause);
}
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
}
}
@@ -396,14 +395,13 @@ static void mwifiex_process_sta_tx_pause(struct mwifiex_private *priv,
if (mwifiex_is_tdls_link_setup(status)) {
spin_lock_irqsave(&priv->sta_list_spinlock, flags);
sta_ptr = mwifiex_get_sta_entry(priv, tp->peermac);
- spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
-
if (sta_ptr && sta_ptr->tx_pause != tp->tx_pause) {
sta_ptr->tx_pause = tp->tx_pause;
mwifiex_update_ralist_tx_pause(priv,
tp->peermac,
tp->tx_pause);
}
+ spin_unlock_irqrestore(&priv->sta_list_spinlock, flags);
}
}
}
--
1.9.1