* [PATCH 1/2] mwifiex: kill useless list_empty checks
@ 2017-10-03 15:19 Ganapathi Bhat
2017-10-03 15:19 ` [PATCH 2/2] mwifiex: minor cleanups w/ sta_list_spinlock in cfg80211.c Ganapathi Bhat
2017-10-13 9:38 ` [1/2] mwifiex: kill useless list_empty checks Kalle Valo
0 siblings, 2 replies; 3+ messages in thread
From: Ganapathi Bhat @ 2017-10-03 15:19 UTC (permalink / raw)
To: linux-wireless
Cc: Brian Norris, Cathy Luo, Xinming Hu, Zhiyuan Yang, James Cao,
Mangesh Malusare, Douglas Anderson, Ganapathi Bhat
From: Douglas Anderson <dianders@chromium.org>
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 <dianders@chromium.org>
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 2/2] mwifiex: minor cleanups w/ sta_list_spinlock in cfg80211.c 2017-10-03 15:19 [PATCH 1/2] mwifiex: kill useless list_empty checks Ganapathi Bhat @ 2017-10-03 15:19 ` Ganapathi Bhat 2017-10-13 9:38 ` [1/2] mwifiex: kill useless list_empty checks Kalle Valo 1 sibling, 0 replies; 3+ messages in thread From: Ganapathi Bhat @ 2017-10-03 15:19 UTC (permalink / raw) To: linux-wireless Cc: Brian Norris, Cathy Luo, Xinming Hu, Zhiyuan Yang, James Cao, Mangesh Malusare, Douglas Anderson, Ganapathi Bhat From: Douglas Anderson <dianders@chromium.org> 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 <dianders@chromium.org> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com> --- 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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [1/2] mwifiex: kill useless list_empty checks 2017-10-03 15:19 [PATCH 1/2] mwifiex: kill useless list_empty checks Ganapathi Bhat 2017-10-03 15:19 ` [PATCH 2/2] mwifiex: minor cleanups w/ sta_list_spinlock in cfg80211.c Ganapathi Bhat @ 2017-10-13 9:38 ` Kalle Valo 1 sibling, 0 replies; 3+ messages in thread From: Kalle Valo @ 2017-10-13 9:38 UTC (permalink / raw) To: Ganapathi Bhat Cc: linux-wireless, Brian Norris, Cathy Luo, Xinming Hu, Zhiyuan Yang, James Cao, Mangesh Malusare, Douglas Anderson, Ganapathi Bhat Ganapathi Bhat <gbhat@marvell.com> wrote: > From: Douglas Anderson <dianders@chromium.org> > > 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 <dianders@chromium.org> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com> 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-13 9:38 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-10-03 15:19 [PATCH 1/2] mwifiex: kill useless list_empty checks Ganapathi Bhat 2017-10-03 15:19 ` [PATCH 2/2] mwifiex: minor cleanups w/ sta_list_spinlock in cfg80211.c Ganapathi Bhat 2017-10-13 9:38 ` [1/2] mwifiex: kill useless list_empty checks Kalle Valo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.