* [PATCH] staging: r8188eu: Use a Mutex instead of a binary Semaphore
@ 2021-10-22 17:19 Fabio M. De Francesco
2021-10-22 17:52 ` Larry Finger
0 siblings, 1 reply; 3+ messages in thread
From: Fabio M. De Francesco @ 2021-10-22 17:19 UTC (permalink / raw)
To: Larry Finger, Phillip Potter, Greg Kroah-Hartman, linux-staging,
linux-kernel
Cc: Fabio M. De Francesco
Use a Mutex instead of a binary Semaphore for the purpose of enforcing
mutual exclusive access to the "pwrctrl_priv" structure.
Mutexes are sleeping locks similar to Semaphores with a 'count' of one
(like binary Semaphores), however they have a simpler interface, more
efficient performance, and additional constraints.
There is no change in the logic of the new code; however it is more
simple because it gets rid of four unnecessary wrappers:
_init_pwrlock(), _enter_pwrlock(),_exit_pwrlock(), _rtw_down_sema().
Actually, there is a change in the state in which the code waits for
acquiring locks, because it makes it in an uninterruptible state
(instead the old code used down_interruptibe()). Interruptible
waits are neither required nor wanted in this driver.
Tested with ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano].
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---
drivers/staging/r8188eu/core/rtw_pwrctrl.c | 10 +++++-----
drivers/staging/r8188eu/include/osdep_service.h | 2 --
drivers/staging/r8188eu/include/rtw_pwrctrl.h | 17 +----------------
drivers/staging/r8188eu/os_dep/osdep_service.c | 8 --------
drivers/staging/r8188eu/os_dep/usb_intf.c | 8 ++++----
5 files changed, 10 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
index 19cac5814ea4..5d595cf2a47e 100644
--- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
+++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
@@ -21,7 +21,7 @@ void ips_enter(struct adapter *padapter)
return;
}
- _enter_pwrlock(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
pwrpriv->bips_processing = true;
@@ -42,7 +42,7 @@ void ips_enter(struct adapter *padapter)
}
pwrpriv->bips_processing = false;
- _exit_pwrlock(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
}
int ips_leave(struct adapter *padapter)
@@ -53,7 +53,7 @@ int ips_leave(struct adapter *padapter)
int result = _SUCCESS;
int keyid;
- _enter_pwrlock(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
if ((pwrpriv->rf_pwrstate == rf_off) && (!pwrpriv->bips_processing)) {
pwrpriv->bips_processing = true;
@@ -87,7 +87,7 @@ int ips_leave(struct adapter *padapter)
pwrpriv->bpower_saving = false;
}
- _exit_pwrlock(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
return result;
}
@@ -337,7 +337,7 @@ void rtw_init_pwrctrl_priv(struct adapter *padapter)
{
struct pwrctrl_priv *pwrctrlpriv = &padapter->pwrctrlpriv;
- _init_pwrlock(&pwrctrlpriv->lock);
+ mutex_init(&pwrctrlpriv->lock);
pwrctrlpriv->rf_pwrstate = rf_on;
pwrctrlpriv->ips_enter_cnts = 0;
pwrctrlpriv->ips_leave_cnts = 0;
diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
index 886a1b6f30b4..efab3a97eb46 100644
--- a/drivers/staging/r8188eu/include/osdep_service.h
+++ b/drivers/staging/r8188eu/include/osdep_service.h
@@ -141,8 +141,6 @@ extern unsigned char RSN_TKIP_CIPHER[4];
void *rtw_malloc2d(int h, int w, int size);
-u32 _rtw_down_sema(struct semaphore *sema);
-
#define rtw_init_queue(q) \
do { \
INIT_LIST_HEAD(&((q)->queue)); \
diff --git a/drivers/staging/r8188eu/include/rtw_pwrctrl.h b/drivers/staging/r8188eu/include/rtw_pwrctrl.h
index 04236e42fbf9..b19ef796ab54 100644
--- a/drivers/staging/r8188eu/include/rtw_pwrctrl.h
+++ b/drivers/staging/r8188eu/include/rtw_pwrctrl.h
@@ -27,21 +27,6 @@ enum power_mgnt {
PS_MODE_NUM
};
-static inline void _init_pwrlock(struct semaphore *plock)
-{
- sema_init(plock, 1);
-}
-
-static inline void _enter_pwrlock(struct semaphore *plock)
-{
- _rtw_down_sema(plock);
-}
-
-static inline void _exit_pwrlock(struct semaphore *plock)
-{
- up(plock);
-}
-
#define LPS_DELAY_TIME 1*HZ /* 1 sec */
/* RF state. */
@@ -60,7 +45,7 @@ enum { /* for ips_mode */
};
struct pwrctrl_priv {
- struct semaphore lock;
+ struct mutex lock; /* Mutex used to protect struct pwrctrl_priv */
u8 pwr_mode;
u8 smart_ps;
diff --git a/drivers/staging/r8188eu/os_dep/osdep_service.c b/drivers/staging/r8188eu/os_dep/osdep_service.c
index 6bee194fc35d..59bdd0abea7e 100644
--- a/drivers/staging/r8188eu/os_dep/osdep_service.c
+++ b/drivers/staging/r8188eu/os_dep/osdep_service.c
@@ -42,14 +42,6 @@ Otherwise, there will be racing condition.
Caller must check if the list is empty before calling rtw_list_delete
*/
-u32 _rtw_down_sema(struct semaphore *sema)
-{
- if (down_interruptible(sema))
- return _FAIL;
- else
- return _SUCCESS;
-}
-
inline u32 rtw_systime_to_ms(u32 systime)
{
return systime * 1000 / HZ;
diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
index 7ed9f5f54472..fc74c93272a8 100644
--- a/drivers/staging/r8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
@@ -233,7 +233,7 @@ static int rtw_suspend(struct usb_interface *pusb_intf, pm_message_t message)
rtw_cancel_all_timer(padapter);
LeaveAllPowerSaveMode(padapter);
- _enter_pwrlock(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
/* s1. */
if (pnetdev) {
netif_carrier_off(pnetdev);
@@ -262,7 +262,7 @@ static int rtw_suspend(struct usb_interface *pusb_intf, pm_message_t message)
rtw_free_network_queue(padapter, true);
rtw_dev_unload(padapter);
- _exit_pwrlock(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
rtw_indicate_scan_done(padapter, 1);
@@ -291,7 +291,7 @@ static int rtw_resume(struct usb_interface *pusb_intf)
pnetdev = padapter->pnetdev;
pwrpriv = &padapter->pwrctrlpriv;
- _enter_pwrlock(&pwrpriv->lock);
+ mutex_lock(&pwrpriv->lock);
rtw_reset_drv_sw(padapter);
if (pwrpriv)
pwrpriv->bkeepfwalive = false;
@@ -303,7 +303,7 @@ static int rtw_resume(struct usb_interface *pusb_intf)
netif_device_attach(pnetdev);
netif_carrier_on(pnetdev);
- _exit_pwrlock(&pwrpriv->lock);
+ mutex_unlock(&pwrpriv->lock);
if (padapter->pid[1] != 0) {
DBG_88E("pid[1]:%d\n", padapter->pid[1]);
--
2.33.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: r8188eu: Use a Mutex instead of a binary Semaphore
2021-10-22 17:19 [PATCH] staging: r8188eu: Use a Mutex instead of a binary Semaphore Fabio M. De Francesco
@ 2021-10-22 17:52 ` Larry Finger
2021-10-23 14:02 ` Fabio M. De Francesco
0 siblings, 1 reply; 3+ messages in thread
From: Larry Finger @ 2021-10-22 17:52 UTC (permalink / raw)
To: Fabio M. De Francesco, Phillip Potter, Greg Kroah-Hartman,
linux-staging, linux-kernel
On 10/22/21 12:19, Fabio M. De Francesco wrote:
> Use a Mutex instead of a binary Semaphore for the purpose of enforcing
> mutual exclusive access to the "pwrctrl_priv" structure.
>
> Mutexes are sleeping locks similar to Semaphores with a 'count' of one
> (like binary Semaphores), however they have a simpler interface, more
> efficient performance, and additional constraints.
>
> There is no change in the logic of the new code; however it is more
> simple because it gets rid of four unnecessary wrappers:
> _init_pwrlock(), _enter_pwrlock(),_exit_pwrlock(), _rtw_down_sema().
>
> Actually, there is a change in the state in which the code waits for
> acquiring locks, because it makes it in an uninterruptible state
> (instead the old code used down_interruptibe()). Interruptible
> waits are neither required nor wanted in this driver.
>
> Tested with ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano].
>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Well done.
Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> drivers/staging/r8188eu/core/rtw_pwrctrl.c | 10 +++++-----
> drivers/staging/r8188eu/include/osdep_service.h | 2 --
> drivers/staging/r8188eu/include/rtw_pwrctrl.h | 17 +----------------
> drivers/staging/r8188eu/os_dep/osdep_service.c | 8 --------
> drivers/staging/r8188eu/os_dep/usb_intf.c | 8 ++++----
> 5 files changed, 10 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_pwrctrl.c b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
> index 19cac5814ea4..5d595cf2a47e 100644
> --- a/drivers/staging/r8188eu/core/rtw_pwrctrl.c
> +++ b/drivers/staging/r8188eu/core/rtw_pwrctrl.c
> @@ -21,7 +21,7 @@ void ips_enter(struct adapter *padapter)
> return;
> }
>
> - _enter_pwrlock(&pwrpriv->lock);
> + mutex_lock(&pwrpriv->lock);
>
> pwrpriv->bips_processing = true;
>
> @@ -42,7 +42,7 @@ void ips_enter(struct adapter *padapter)
> }
> pwrpriv->bips_processing = false;
>
> - _exit_pwrlock(&pwrpriv->lock);
> + mutex_unlock(&pwrpriv->lock);
> }
>
> int ips_leave(struct adapter *padapter)
> @@ -53,7 +53,7 @@ int ips_leave(struct adapter *padapter)
> int result = _SUCCESS;
> int keyid;
>
> - _enter_pwrlock(&pwrpriv->lock);
> + mutex_lock(&pwrpriv->lock);
>
> if ((pwrpriv->rf_pwrstate == rf_off) && (!pwrpriv->bips_processing)) {
> pwrpriv->bips_processing = true;
> @@ -87,7 +87,7 @@ int ips_leave(struct adapter *padapter)
> pwrpriv->bpower_saving = false;
> }
>
> - _exit_pwrlock(&pwrpriv->lock);
> + mutex_unlock(&pwrpriv->lock);
>
> return result;
> }
> @@ -337,7 +337,7 @@ void rtw_init_pwrctrl_priv(struct adapter *padapter)
> {
> struct pwrctrl_priv *pwrctrlpriv = &padapter->pwrctrlpriv;
>
> - _init_pwrlock(&pwrctrlpriv->lock);
> + mutex_init(&pwrctrlpriv->lock);
> pwrctrlpriv->rf_pwrstate = rf_on;
> pwrctrlpriv->ips_enter_cnts = 0;
> pwrctrlpriv->ips_leave_cnts = 0;
> diff --git a/drivers/staging/r8188eu/include/osdep_service.h b/drivers/staging/r8188eu/include/osdep_service.h
> index 886a1b6f30b4..efab3a97eb46 100644
> --- a/drivers/staging/r8188eu/include/osdep_service.h
> +++ b/drivers/staging/r8188eu/include/osdep_service.h
> @@ -141,8 +141,6 @@ extern unsigned char RSN_TKIP_CIPHER[4];
>
> void *rtw_malloc2d(int h, int w, int size);
>
> -u32 _rtw_down_sema(struct semaphore *sema);
> -
> #define rtw_init_queue(q) \
> do { \
> INIT_LIST_HEAD(&((q)->queue)); \
> diff --git a/drivers/staging/r8188eu/include/rtw_pwrctrl.h b/drivers/staging/r8188eu/include/rtw_pwrctrl.h
> index 04236e42fbf9..b19ef796ab54 100644
> --- a/drivers/staging/r8188eu/include/rtw_pwrctrl.h
> +++ b/drivers/staging/r8188eu/include/rtw_pwrctrl.h
> @@ -27,21 +27,6 @@ enum power_mgnt {
> PS_MODE_NUM
> };
>
> -static inline void _init_pwrlock(struct semaphore *plock)
> -{
> - sema_init(plock, 1);
> -}
> -
> -static inline void _enter_pwrlock(struct semaphore *plock)
> -{
> - _rtw_down_sema(plock);
> -}
> -
> -static inline void _exit_pwrlock(struct semaphore *plock)
> -{
> - up(plock);
> -}
> -
> #define LPS_DELAY_TIME 1*HZ /* 1 sec */
>
> /* RF state. */
> @@ -60,7 +45,7 @@ enum { /* for ips_mode */
> };
>
> struct pwrctrl_priv {
> - struct semaphore lock;
> + struct mutex lock; /* Mutex used to protect struct pwrctrl_priv */
>
> u8 pwr_mode;
> u8 smart_ps;
> diff --git a/drivers/staging/r8188eu/os_dep/osdep_service.c b/drivers/staging/r8188eu/os_dep/osdep_service.c
> index 6bee194fc35d..59bdd0abea7e 100644
> --- a/drivers/staging/r8188eu/os_dep/osdep_service.c
> +++ b/drivers/staging/r8188eu/os_dep/osdep_service.c
> @@ -42,14 +42,6 @@ Otherwise, there will be racing condition.
> Caller must check if the list is empty before calling rtw_list_delete
> */
>
> -u32 _rtw_down_sema(struct semaphore *sema)
> -{
> - if (down_interruptible(sema))
> - return _FAIL;
> - else
> - return _SUCCESS;
> -}
> -
> inline u32 rtw_systime_to_ms(u32 systime)
> {
> return systime * 1000 / HZ;
> diff --git a/drivers/staging/r8188eu/os_dep/usb_intf.c b/drivers/staging/r8188eu/os_dep/usb_intf.c
> index 7ed9f5f54472..fc74c93272a8 100644
> --- a/drivers/staging/r8188eu/os_dep/usb_intf.c
> +++ b/drivers/staging/r8188eu/os_dep/usb_intf.c
> @@ -233,7 +233,7 @@ static int rtw_suspend(struct usb_interface *pusb_intf, pm_message_t message)
> rtw_cancel_all_timer(padapter);
> LeaveAllPowerSaveMode(padapter);
>
> - _enter_pwrlock(&pwrpriv->lock);
> + mutex_lock(&pwrpriv->lock);
> /* s1. */
> if (pnetdev) {
> netif_carrier_off(pnetdev);
> @@ -262,7 +262,7 @@ static int rtw_suspend(struct usb_interface *pusb_intf, pm_message_t message)
> rtw_free_network_queue(padapter, true);
>
> rtw_dev_unload(padapter);
> - _exit_pwrlock(&pwrpriv->lock);
> + mutex_unlock(&pwrpriv->lock);
>
> if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY))
> rtw_indicate_scan_done(padapter, 1);
> @@ -291,7 +291,7 @@ static int rtw_resume(struct usb_interface *pusb_intf)
> pnetdev = padapter->pnetdev;
> pwrpriv = &padapter->pwrctrlpriv;
>
> - _enter_pwrlock(&pwrpriv->lock);
> + mutex_lock(&pwrpriv->lock);
> rtw_reset_drv_sw(padapter);
> if (pwrpriv)
> pwrpriv->bkeepfwalive = false;
> @@ -303,7 +303,7 @@ static int rtw_resume(struct usb_interface *pusb_intf)
> netif_device_attach(pnetdev);
> netif_carrier_on(pnetdev);
>
> - _exit_pwrlock(&pwrpriv->lock);
> + mutex_unlock(&pwrpriv->lock);
>
> if (padapter->pid[1] != 0) {
> DBG_88E("pid[1]:%d\n", padapter->pid[1]);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: r8188eu: Use a Mutex instead of a binary Semaphore
2021-10-22 17:52 ` Larry Finger
@ 2021-10-23 14:02 ` Fabio M. De Francesco
0 siblings, 0 replies; 3+ messages in thread
From: Fabio M. De Francesco @ 2021-10-23 14:02 UTC (permalink / raw)
To: Phillip Potter, Greg Kroah-Hartman, linux-staging, linux-kernel,
Larry Finger
On Friday, October 22, 2021 7:52:33 PM CEST Larry Finger wrote:
> On 10/22/21 12:19, Fabio M. De Francesco wrote:
> > Use a Mutex instead of a binary Semaphore for the purpose of enforcing
> > mutual exclusive access to the "pwrctrl_priv" structure.
> >
> > Mutexes are sleeping locks similar to Semaphores with a 'count' of one
> > (like binary Semaphores), however they have a simpler interface, more
> > efficient performance, and additional constraints.
> >
> > There is no change in the logic of the new code; however it is more
> > simple because it gets rid of four unnecessary wrappers:
> > _init_pwrlock(), _enter_pwrlock(),_exit_pwrlock(), _rtw_down_sema().
> >
> > Actually, there is a change in the state in which the code waits for
> > acquiring locks, because it makes it in an uninterruptible state
> > (instead the old code used down_interruptible()). Interruptible
> > waits are neither required nor wanted in this driver.
> >
> > Tested with ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano].
> >
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>
> Well done.
>
> Acked-by: Larry Finger <Larry.Finger@lwfinger.net>
>
Hi Larry,
Thank you very much for giving your "Acked-by" tag, and, above all, for the
"Well done".
Best regards,
Fabio
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-23 14:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-10-22 17:19 [PATCH] staging: r8188eu: Use a Mutex instead of a binary Semaphore Fabio M. De Francesco
2021-10-22 17:52 ` Larry Finger
2021-10-23 14:02 ` Fabio M. De Francesco
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.