2011-12-12 11:43:55

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 1/2] rtlwifi: use work for lps

Leaving leisure power save mode can take some time, so it's better to
perform that action in process context with interrupts enabled. This
patch changes lps_leave tasklet to work.

Reported-by: Philipp Dreimann <[email protected]>
Tested-by: Larry Finger <[email protected]>
Cc: Mike McCormack <[email protected]>
Cc: Chaoming Li <[email protected]>
Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/rtlwifi/pci.c | 26 ++++++++++++++------------
drivers/net/wireless/rtlwifi/wifi.h | 3 ++-
2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
index 91f0525..0d4d242 100644
--- a/drivers/net/wireless/rtlwifi/pci.c
+++ b/drivers/net/wireless/rtlwifi/pci.c
@@ -610,7 +610,7 @@ tx_status_ok:
if (((rtlpriv->link_info.num_rx_inperiod +
rtlpriv->link_info.num_tx_inperiod) > 8) ||
(rtlpriv->link_info.num_rx_inperiod > 2)) {
- tasklet_schedule(&rtlpriv->works.ips_leave_tasklet);
+ schedule_work(&rtlpriv->works.lps_leave_work);
}
}

@@ -736,7 +736,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
if (((rtlpriv->link_info.num_rx_inperiod +
rtlpriv->link_info.num_tx_inperiod) > 8) ||
(rtlpriv->link_info.num_rx_inperiod > 2)) {
- tasklet_schedule(&rtlpriv->works.ips_leave_tasklet);
+ schedule_work(&rtlpriv->works.lps_leave_work);
}

dev_kfree_skb_any(skb);
@@ -903,11 +903,6 @@ static void _rtl_pci_irq_tasklet(struct ieee80211_hw *hw)
_rtl_pci_tx_chk_waitq(hw);
}

-static void _rtl_pci_ips_leave_tasklet(struct ieee80211_hw *hw)
-{
- rtl_lps_leave(hw);
-}
-
static void _rtl_pci_prepare_bcn_tasklet(struct ieee80211_hw *hw)
{
struct rtl_priv *rtlpriv = rtl_priv(hw);
@@ -945,6 +940,15 @@ static void _rtl_pci_prepare_bcn_tasklet(struct ieee80211_hw *hw)
return;
}

+static void rtl_lps_leave_work_callback(struct work_struct *work)
+{
+ struct rtl_works *rtlworks =
+ container_of(work, struct rtl_works, lps_leave_work);
+ struct ieee80211_hw *hw = rtlworks->hw;
+
+ rtl_lps_leave(hw);
+}
+
static void _rtl_pci_init_trx_var(struct ieee80211_hw *hw)
{
struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
@@ -1006,9 +1010,7 @@ static void _rtl_pci_init_struct(struct ieee80211_hw *hw,
tasklet_init(&rtlpriv->works.irq_prepare_bcn_tasklet,
(void (*)(unsigned long))_rtl_pci_prepare_bcn_tasklet,
(unsigned long)hw);
- tasklet_init(&rtlpriv->works.ips_leave_tasklet,
- (void (*)(unsigned long))_rtl_pci_ips_leave_tasklet,
- (unsigned long)hw);
+ INIT_WORK(&rtlpriv->works.lps_leave_work, rtl_lps_leave_work_callback);
}

static int _rtl_pci_init_tx_ring(struct ieee80211_hw *hw,
@@ -1478,7 +1480,7 @@ static void rtl_pci_deinit(struct ieee80211_hw *hw)

synchronize_irq(rtlpci->pdev->irq);
tasklet_kill(&rtlpriv->works.irq_tasklet);
- tasklet_kill(&rtlpriv->works.ips_leave_tasklet);
+ cancel_work_sync(&rtlpriv->works.lps_leave_work);

flush_workqueue(rtlpriv->works.rtl_wq);
destroy_workqueue(rtlpriv->works.rtl_wq);
@@ -1553,7 +1555,7 @@ static void rtl_pci_stop(struct ieee80211_hw *hw)
set_hal_stop(rtlhal);

rtlpriv->cfg->ops->disable_interrupt(hw);
- tasklet_kill(&rtlpriv->works.ips_leave_tasklet);
+ cancel_work_sync(&rtlpriv->works.lps_leave_work);

spin_lock_irqsave(&rtlpriv->locks.rf_ps_lock, flags);
while (ppsc->rfchange_inprogress) {
diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h
index f3c132b..6e6353b 100644
--- a/drivers/net/wireless/rtlwifi/wifi.h
+++ b/drivers/net/wireless/rtlwifi/wifi.h
@@ -1576,7 +1576,8 @@ struct rtl_works {
/* For SW LPS */
struct delayed_work ps_work;
struct delayed_work ps_rfon_wq;
- struct tasklet_struct ips_leave_tasklet;
+
+ struct work_struct lps_leave_work;
};

struct rtl_debug {
--
1.7.1



2011-12-12 11:44:09

by Stanislaw Gruszka

[permalink] [raw]
Subject: [PATCH 2/2] rtlwifi: merge ips,lps spinlocks into one mutex

With previous patch "rtlwifi: use work for lps" we can now use mutex for
protecting ps mode changing critical sections. This fixes running system
with interrupts disabled for long time.

Merge ips_lock and lps_lock as they seems to protect the same data
structures (accessed in rtl_ps_set_rf_state() function).

Reported-by: Philipp Dreimann <[email protected]>
Tested-by: Larry Finger <[email protected]>
Cc: Mike McCormack <[email protected]>
Cc: Chaoming Li <[email protected]>
Signed-off-by: Stanislaw Gruszka <[email protected]>
---
drivers/net/wireless/rtlwifi/base.c | 3 +--
drivers/net/wireless/rtlwifi/ps.c | 21 ++++++++++-----------
drivers/net/wireless/rtlwifi/wifi.h | 3 +--
3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
index a13ecfc..d81a602 100644
--- a/drivers/net/wireless/rtlwifi/base.c
+++ b/drivers/net/wireless/rtlwifi/base.c
@@ -448,12 +448,11 @@ int rtl_init_core(struct ieee80211_hw *hw)

/* <4> locks */
mutex_init(&rtlpriv->locks.conf_mutex);
- spin_lock_init(&rtlpriv->locks.ips_lock);
+ mutex_init(&rtlpriv->locks.ps_mutex);
spin_lock_init(&rtlpriv->locks.irq_th_lock);
spin_lock_init(&rtlpriv->locks.h2c_lock);
spin_lock_init(&rtlpriv->locks.rf_ps_lock);
spin_lock_init(&rtlpriv->locks.rf_lock);
- spin_lock_init(&rtlpriv->locks.lps_lock);
spin_lock_init(&rtlpriv->locks.waitq_lock);
spin_lock_init(&rtlpriv->locks.cck_and_rw_pagea_lock);

diff --git a/drivers/net/wireless/rtlwifi/ps.c b/drivers/net/wireless/rtlwifi/ps.c
index 55c8e50..a14a68b 100644
--- a/drivers/net/wireless/rtlwifi/ps.c
+++ b/drivers/net/wireless/rtlwifi/ps.c
@@ -241,7 +241,7 @@ void rtl_ips_nic_on(struct ieee80211_hw *hw)
if (mac->opmode != NL80211_IFTYPE_STATION)
return;

- spin_lock(&rtlpriv->locks.ips_lock);
+ mutex_lock(&rtlpriv->locks.ps_mutex);

if (ppsc->inactiveps) {
rtstate = ppsc->rfpwr_state;
@@ -257,7 +257,7 @@ void rtl_ips_nic_on(struct ieee80211_hw *hw)
}
}

- spin_unlock(&rtlpriv->locks.ips_lock);
+ mutex_unlock(&rtlpriv->locks.ps_mutex);
}

/*for FW LPS*/
@@ -395,7 +395,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw)
if (mac->link_state != MAC80211_LINKED)
return;

- spin_lock_irq(&rtlpriv->locks.lps_lock);
+ mutex_lock(&rtlpriv->locks.ps_mutex);

/* Idle for a while if we connect to AP a while ago. */
if (mac->cnt_after_linked >= 2) {
@@ -407,7 +407,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw)
}
}

- spin_unlock_irq(&rtlpriv->locks.lps_lock);
+ mutex_unlock(&rtlpriv->locks.ps_mutex);
}

/*Leave the leisure power save mode.*/
@@ -416,9 +416,8 @@ void rtl_lps_leave(struct ieee80211_hw *hw)
struct rtl_priv *rtlpriv = rtl_priv(hw);
struct rtl_ps_ctl *ppsc = rtl_psc(rtl_priv(hw));
struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
- unsigned long flags;

- spin_lock_irqsave(&rtlpriv->locks.lps_lock, flags);
+ mutex_lock(&rtlpriv->locks.ps_mutex);

if (ppsc->fwctrl_lps) {
if (ppsc->dot11_psmode != EACTIVE) {
@@ -439,7 +438,7 @@ void rtl_lps_leave(struct ieee80211_hw *hw)
rtl_lps_set_psmode(hw, EACTIVE);
}
}
- spin_unlock_irqrestore(&rtlpriv->locks.lps_lock, flags);
+ mutex_unlock(&rtlpriv->locks.ps_mutex);
}

/* For sw LPS*/
@@ -540,9 +539,9 @@ void rtl_swlps_rf_awake(struct ieee80211_hw *hw)
RT_CLEAR_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM);
}

- spin_lock_irq(&rtlpriv->locks.lps_lock);
+ mutex_lock(&rtlpriv->locks.ps_mutex);
rtl_ps_set_rf_state(hw, ERFON, RF_CHANGE_BY_PS);
- spin_unlock_irq(&rtlpriv->locks.lps_lock);
+ mutex_unlock(&rtlpriv->locks.ps_mutex);
}

void rtl_swlps_rfon_wq_callback(void *data)
@@ -575,9 +574,9 @@ void rtl_swlps_rf_sleep(struct ieee80211_hw *hw)
if (rtlpriv->link_info.busytraffic)
return;

- spin_lock_irq(&rtlpriv->locks.lps_lock);
+ mutex_lock(&rtlpriv->locks.ps_mutex);
rtl_ps_set_rf_state(hw, ERFSLEEP, RF_CHANGE_BY_PS);
- spin_unlock_irq(&rtlpriv->locks.lps_lock);
+ mutex_unlock(&rtlpriv->locks.ps_mutex);

if (ppsc->reg_rfps_level & RT_RF_OFF_LEVL_ASPM &&
!RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM)) {
diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h
index 6e6353b..085dccd 100644
--- a/drivers/net/wireless/rtlwifi/wifi.h
+++ b/drivers/net/wireless/rtlwifi/wifi.h
@@ -1544,14 +1544,13 @@ struct rtl_hal_cfg {
struct rtl_locks {
/* mutex */
struct mutex conf_mutex;
+ struct mutex ps_mutex;

/*spin lock */
- spinlock_t ips_lock;
spinlock_t irq_th_lock;
spinlock_t h2c_lock;
spinlock_t rf_ps_lock;
spinlock_t rf_lock;
- spinlock_t lps_lock;
spinlock_t waitq_lock;

/*Dual mac*/
--
1.7.1


2011-12-12 15:54:23

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 1/2] rtlwifi: use work for lps

On 12/12/2011 05:43 AM, Stanislaw Gruszka wrote:
> Leaving leisure power save mode can take some time, so it's better to
> perform that action in process context with interrupts enabled. This
> patch changes lps_leave tasklet to work.
>
> Reported-by: Philipp Dreimann<[email protected]>
> Tested-by: Larry Finger<[email protected]>
> Cc: Mike McCormack<[email protected]>
> Cc: Chaoming Li<[email protected]>
> Signed-off-by: Stanislaw Gruszka<[email protected]>

Signed-off-by: Larry Finger <[email protected]>


> ---
> drivers/net/wireless/rtlwifi/pci.c | 26 ++++++++++++++------------
> drivers/net/wireless/rtlwifi/wifi.h | 3 ++-
> 2 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/pci.c b/drivers/net/wireless/rtlwifi/pci.c
> index 91f0525..0d4d242 100644
> --- a/drivers/net/wireless/rtlwifi/pci.c
> +++ b/drivers/net/wireless/rtlwifi/pci.c
> @@ -610,7 +610,7 @@ tx_status_ok:
> if (((rtlpriv->link_info.num_rx_inperiod +
> rtlpriv->link_info.num_tx_inperiod)> 8) ||
> (rtlpriv->link_info.num_rx_inperiod> 2)) {
> - tasklet_schedule(&rtlpriv->works.ips_leave_tasklet);
> + schedule_work(&rtlpriv->works.lps_leave_work);
> }
> }
>
> @@ -736,7 +736,7 @@ static void _rtl_pci_rx_interrupt(struct ieee80211_hw *hw)
> if (((rtlpriv->link_info.num_rx_inperiod +
> rtlpriv->link_info.num_tx_inperiod)> 8) ||
> (rtlpriv->link_info.num_rx_inperiod> 2)) {
> - tasklet_schedule(&rtlpriv->works.ips_leave_tasklet);
> + schedule_work(&rtlpriv->works.lps_leave_work);
> }
>
> dev_kfree_skb_any(skb);
> @@ -903,11 +903,6 @@ static void _rtl_pci_irq_tasklet(struct ieee80211_hw *hw)
> _rtl_pci_tx_chk_waitq(hw);
> }
>
> -static void _rtl_pci_ips_leave_tasklet(struct ieee80211_hw *hw)
> -{
> - rtl_lps_leave(hw);
> -}
> -
> static void _rtl_pci_prepare_bcn_tasklet(struct ieee80211_hw *hw)
> {
> struct rtl_priv *rtlpriv = rtl_priv(hw);
> @@ -945,6 +940,15 @@ static void _rtl_pci_prepare_bcn_tasklet(struct ieee80211_hw *hw)
> return;
> }
>
> +static void rtl_lps_leave_work_callback(struct work_struct *work)
> +{
> + struct rtl_works *rtlworks =
> + container_of(work, struct rtl_works, lps_leave_work);
> + struct ieee80211_hw *hw = rtlworks->hw;
> +
> + rtl_lps_leave(hw);
> +}
> +
> static void _rtl_pci_init_trx_var(struct ieee80211_hw *hw)
> {
> struct rtl_pci *rtlpci = rtl_pcidev(rtl_pcipriv(hw));
> @@ -1006,9 +1010,7 @@ static void _rtl_pci_init_struct(struct ieee80211_hw *hw,
> tasklet_init(&rtlpriv->works.irq_prepare_bcn_tasklet,
> (void (*)(unsigned long))_rtl_pci_prepare_bcn_tasklet,
> (unsigned long)hw);
> - tasklet_init(&rtlpriv->works.ips_leave_tasklet,
> - (void (*)(unsigned long))_rtl_pci_ips_leave_tasklet,
> - (unsigned long)hw);
> + INIT_WORK(&rtlpriv->works.lps_leave_work, rtl_lps_leave_work_callback);
> }
>
> static int _rtl_pci_init_tx_ring(struct ieee80211_hw *hw,
> @@ -1478,7 +1480,7 @@ static void rtl_pci_deinit(struct ieee80211_hw *hw)
>
> synchronize_irq(rtlpci->pdev->irq);
> tasklet_kill(&rtlpriv->works.irq_tasklet);
> - tasklet_kill(&rtlpriv->works.ips_leave_tasklet);
> + cancel_work_sync(&rtlpriv->works.lps_leave_work);
>
> flush_workqueue(rtlpriv->works.rtl_wq);
> destroy_workqueue(rtlpriv->works.rtl_wq);
> @@ -1553,7 +1555,7 @@ static void rtl_pci_stop(struct ieee80211_hw *hw)
> set_hal_stop(rtlhal);
>
> rtlpriv->cfg->ops->disable_interrupt(hw);
> - tasklet_kill(&rtlpriv->works.ips_leave_tasklet);
> + cancel_work_sync(&rtlpriv->works.lps_leave_work);
>
> spin_lock_irqsave(&rtlpriv->locks.rf_ps_lock, flags);
> while (ppsc->rfchange_inprogress) {
> diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h
> index f3c132b..6e6353b 100644
> --- a/drivers/net/wireless/rtlwifi/wifi.h
> +++ b/drivers/net/wireless/rtlwifi/wifi.h
> @@ -1576,7 +1576,8 @@ struct rtl_works {
> /* For SW LPS */
> struct delayed_work ps_work;
> struct delayed_work ps_rfon_wq;
> - struct tasklet_struct ips_leave_tasklet;
> +
> + struct work_struct lps_leave_work;
> };
>
> struct rtl_debug {


2011-12-12 17:57:04

by Tim Gardner

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtlwifi: merge ips,lps spinlocks into one mutex

On 12/12/2011 04:43 AM, Stanislaw Gruszka wrote:
> With previous patch "rtlwifi: use work for lps" we can now use mutex for
> protecting ps mode changing critical sections. This fixes running system
> with interrupts disabled for long time.
>
> Merge ips_lock and lps_lock as they seems to protect the same data
> structures (accessed in rtl_ps_set_rf_state() function).
>
> Reported-by: Philipp Dreimann<[email protected]>
> Tested-by: Larry Finger<[email protected]>
> Cc: Mike McCormack<[email protected]>
> Cc: Chaoming Li<[email protected]>
> Signed-off-by: Stanislaw Gruszka<[email protected]>
> ---
> drivers/net/wireless/rtlwifi/base.c | 3 +--
> drivers/net/wireless/rtlwifi/ps.c | 21 ++++++++++-----------
> drivers/net/wireless/rtlwifi/wifi.h | 3 +--
> 3 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
> index a13ecfc..d81a602 100644
> --- a/drivers/net/wireless/rtlwifi/base.c
> +++ b/drivers/net/wireless/rtlwifi/base.c
> @@ -448,12 +448,11 @@ int rtl_init_core(struct ieee80211_hw *hw)
>
> /*<4> locks */
> mutex_init(&rtlpriv->locks.conf_mutex);
> - spin_lock_init(&rtlpriv->locks.ips_lock);
> + mutex_init(&rtlpriv->locks.ps_mutex);
> spin_lock_init(&rtlpriv->locks.irq_th_lock);
> spin_lock_init(&rtlpriv->locks.h2c_lock);
> spin_lock_init(&rtlpriv->locks.rf_ps_lock);
> spin_lock_init(&rtlpriv->locks.rf_lock);
> - spin_lock_init(&rtlpriv->locks.lps_lock);
> spin_lock_init(&rtlpriv->locks.waitq_lock);
> spin_lock_init(&rtlpriv->locks.cck_and_rw_pagea_lock);
>
> diff --git a/drivers/net/wireless/rtlwifi/ps.c b/drivers/net/wireless/rtlwifi/ps.c
> index 55c8e50..a14a68b 100644
> --- a/drivers/net/wireless/rtlwifi/ps.c
> +++ b/drivers/net/wireless/rtlwifi/ps.c
> @@ -241,7 +241,7 @@ void rtl_ips_nic_on(struct ieee80211_hw *hw)
> if (mac->opmode != NL80211_IFTYPE_STATION)
> return;
>
> - spin_lock(&rtlpriv->locks.ips_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
>
> if (ppsc->inactiveps) {
> rtstate = ppsc->rfpwr_state;
> @@ -257,7 +257,7 @@ void rtl_ips_nic_on(struct ieee80211_hw *hw)
> }
> }
>
> - spin_unlock(&rtlpriv->locks.ips_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> /*for FW LPS*/
> @@ -395,7 +395,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw)
> if (mac->link_state != MAC80211_LINKED)
> return;
>
> - spin_lock_irq(&rtlpriv->locks.lps_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
>
> /* Idle for a while if we connect to AP a while ago. */
> if (mac->cnt_after_linked>= 2) {
> @@ -407,7 +407,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw)
> }
> }
>
> - spin_unlock_irq(&rtlpriv->locks.lps_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> /*Leave the leisure power save mode.*/
> @@ -416,9 +416,8 @@ void rtl_lps_leave(struct ieee80211_hw *hw)
> struct rtl_priv *rtlpriv = rtl_priv(hw);
> struct rtl_ps_ctl *ppsc = rtl_psc(rtl_priv(hw));
> struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
> - unsigned long flags;
>
> - spin_lock_irqsave(&rtlpriv->locks.lps_lock, flags);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
>
> if (ppsc->fwctrl_lps) {
> if (ppsc->dot11_psmode != EACTIVE) {
> @@ -439,7 +438,7 @@ void rtl_lps_leave(struct ieee80211_hw *hw)
> rtl_lps_set_psmode(hw, EACTIVE);
> }
> }
> - spin_unlock_irqrestore(&rtlpriv->locks.lps_lock, flags);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> /* For sw LPS*/
> @@ -540,9 +539,9 @@ void rtl_swlps_rf_awake(struct ieee80211_hw *hw)
> RT_CLEAR_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM);
> }
>
> - spin_lock_irq(&rtlpriv->locks.lps_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
> rtl_ps_set_rf_state(hw, ERFON, RF_CHANGE_BY_PS);
> - spin_unlock_irq(&rtlpriv->locks.lps_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> void rtl_swlps_rfon_wq_callback(void *data)
> @@ -575,9 +574,9 @@ void rtl_swlps_rf_sleep(struct ieee80211_hw *hw)
> if (rtlpriv->link_info.busytraffic)
> return;
>
> - spin_lock_irq(&rtlpriv->locks.lps_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
> rtl_ps_set_rf_state(hw, ERFSLEEP, RF_CHANGE_BY_PS);
> - spin_unlock_irq(&rtlpriv->locks.lps_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
>
> if (ppsc->reg_rfps_level& RT_RF_OFF_LEVL_ASPM&&
> !RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM)) {
> diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h
> index 6e6353b..085dccd 100644
> --- a/drivers/net/wireless/rtlwifi/wifi.h
> +++ b/drivers/net/wireless/rtlwifi/wifi.h
> @@ -1544,14 +1544,13 @@ struct rtl_hal_cfg {
> struct rtl_locks {
> /* mutex */
> struct mutex conf_mutex;
> + struct mutex ps_mutex;
>
> /*spin lock */
> - spinlock_t ips_lock;
> spinlock_t irq_th_lock;
> spinlock_t h2c_lock;
> spinlock_t rf_ps_lock;
> spinlock_t rf_lock;
> - spinlock_t lps_lock;
> spinlock_t waitq_lock;
>
> /*Dual mac*/

On an SMP platform spin_lock_irqsave() is only effective if the
interrupt handler uses the same spinlock. Its benign in this case, but
still bogus.

Tested-by: Tim Gardner <[email protected]>
--
Tim Gardner [email protected]

2011-12-12 15:54:58

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH 2/2] rtlwifi: merge ips,lps spinlocks into one mutex

On 12/12/2011 05:43 AM, Stanislaw Gruszka wrote:
> With previous patch "rtlwifi: use work for lps" we can now use mutex for
> protecting ps mode changing critical sections. This fixes running system
> with interrupts disabled for long time.
>
> Merge ips_lock and lps_lock as they seems to protect the same data
> structures (accessed in rtl_ps_set_rf_state() function).
>
> Reported-by: Philipp Dreimann<[email protected]>
> Tested-by: Larry Finger<[email protected]>
> Cc: Mike McCormack<[email protected]>
> Cc: Chaoming Li<[email protected]>
> Signed-off-by: Stanislaw Gruszka<[email protected]>

Signed-off-by: Larry Finger <[email protected]>


> ---
> drivers/net/wireless/rtlwifi/base.c | 3 +--
> drivers/net/wireless/rtlwifi/ps.c | 21 ++++++++++-----------
> drivers/net/wireless/rtlwifi/wifi.h | 3 +--
> 3 files changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/wireless/rtlwifi/base.c b/drivers/net/wireless/rtlwifi/base.c
> index a13ecfc..d81a602 100644
> --- a/drivers/net/wireless/rtlwifi/base.c
> +++ b/drivers/net/wireless/rtlwifi/base.c
> @@ -448,12 +448,11 @@ int rtl_init_core(struct ieee80211_hw *hw)
>
> /*<4> locks */
> mutex_init(&rtlpriv->locks.conf_mutex);
> - spin_lock_init(&rtlpriv->locks.ips_lock);
> + mutex_init(&rtlpriv->locks.ps_mutex);
> spin_lock_init(&rtlpriv->locks.irq_th_lock);
> spin_lock_init(&rtlpriv->locks.h2c_lock);
> spin_lock_init(&rtlpriv->locks.rf_ps_lock);
> spin_lock_init(&rtlpriv->locks.rf_lock);
> - spin_lock_init(&rtlpriv->locks.lps_lock);
> spin_lock_init(&rtlpriv->locks.waitq_lock);
> spin_lock_init(&rtlpriv->locks.cck_and_rw_pagea_lock);
>
> diff --git a/drivers/net/wireless/rtlwifi/ps.c b/drivers/net/wireless/rtlwifi/ps.c
> index 55c8e50..a14a68b 100644
> --- a/drivers/net/wireless/rtlwifi/ps.c
> +++ b/drivers/net/wireless/rtlwifi/ps.c
> @@ -241,7 +241,7 @@ void rtl_ips_nic_on(struct ieee80211_hw *hw)
> if (mac->opmode != NL80211_IFTYPE_STATION)
> return;
>
> - spin_lock(&rtlpriv->locks.ips_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
>
> if (ppsc->inactiveps) {
> rtstate = ppsc->rfpwr_state;
> @@ -257,7 +257,7 @@ void rtl_ips_nic_on(struct ieee80211_hw *hw)
> }
> }
>
> - spin_unlock(&rtlpriv->locks.ips_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> /*for FW LPS*/
> @@ -395,7 +395,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw)
> if (mac->link_state != MAC80211_LINKED)
> return;
>
> - spin_lock_irq(&rtlpriv->locks.lps_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
>
> /* Idle for a while if we connect to AP a while ago. */
> if (mac->cnt_after_linked>= 2) {
> @@ -407,7 +407,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw)
> }
> }
>
> - spin_unlock_irq(&rtlpriv->locks.lps_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> /*Leave the leisure power save mode.*/
> @@ -416,9 +416,8 @@ void rtl_lps_leave(struct ieee80211_hw *hw)
> struct rtl_priv *rtlpriv = rtl_priv(hw);
> struct rtl_ps_ctl *ppsc = rtl_psc(rtl_priv(hw));
> struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw));
> - unsigned long flags;
>
> - spin_lock_irqsave(&rtlpriv->locks.lps_lock, flags);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
>
> if (ppsc->fwctrl_lps) {
> if (ppsc->dot11_psmode != EACTIVE) {
> @@ -439,7 +438,7 @@ void rtl_lps_leave(struct ieee80211_hw *hw)
> rtl_lps_set_psmode(hw, EACTIVE);
> }
> }
> - spin_unlock_irqrestore(&rtlpriv->locks.lps_lock, flags);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> /* For sw LPS*/
> @@ -540,9 +539,9 @@ void rtl_swlps_rf_awake(struct ieee80211_hw *hw)
> RT_CLEAR_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM);
> }
>
> - spin_lock_irq(&rtlpriv->locks.lps_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
> rtl_ps_set_rf_state(hw, ERFON, RF_CHANGE_BY_PS);
> - spin_unlock_irq(&rtlpriv->locks.lps_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
> }
>
> void rtl_swlps_rfon_wq_callback(void *data)
> @@ -575,9 +574,9 @@ void rtl_swlps_rf_sleep(struct ieee80211_hw *hw)
> if (rtlpriv->link_info.busytraffic)
> return;
>
> - spin_lock_irq(&rtlpriv->locks.lps_lock);
> + mutex_lock(&rtlpriv->locks.ps_mutex);
> rtl_ps_set_rf_state(hw, ERFSLEEP, RF_CHANGE_BY_PS);
> - spin_unlock_irq(&rtlpriv->locks.lps_lock);
> + mutex_unlock(&rtlpriv->locks.ps_mutex);
>
> if (ppsc->reg_rfps_level& RT_RF_OFF_LEVL_ASPM&&
> !RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM)) {
> diff --git a/drivers/net/wireless/rtlwifi/wifi.h b/drivers/net/wireless/rtlwifi/wifi.h
> index 6e6353b..085dccd 100644
> --- a/drivers/net/wireless/rtlwifi/wifi.h
> +++ b/drivers/net/wireless/rtlwifi/wifi.h
> @@ -1544,14 +1544,13 @@ struct rtl_hal_cfg {
> struct rtl_locks {
> /* mutex */
> struct mutex conf_mutex;
> + struct mutex ps_mutex;
>
> /*spin lock */
> - spinlock_t ips_lock;
> spinlock_t irq_th_lock;
> spinlock_t h2c_lock;
> spinlock_t rf_ps_lock;
> spinlock_t rf_lock;
> - spinlock_t lps_lock;
> spinlock_t waitq_lock;
>
> /*Dual mac*/