staging: r8188eu: Use completions for signaling start / end kthread

rtw_cmd_thread() "up(s)" a semaphore twice, first to notify callers when
its execution is started and then to notify when it is about to end.

It makes the same semaphore go "up" twice in the same thread. This
construct makes Smatch to warn of duplicate "up(s)".

This thread uses interruptible semaphores where instead completions are
more suitable. For this purpose it calls an helper (_rtw_down_sema())
that returns values that are never checked. It may lead to bugs.

To address the above-mentioned issues, use two completions variables
instead of semaphores. Use the uninterruptible versions of
wake_for_completion*() because the interruptible / killable versions are
not necessary.

Tested with "ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano]".

Acked-by: Phillip Potter <phil@philpotter.co.uk>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Link: https://lore.kernel.org/r/20211018162006.5527-2-fmdefrancesco@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/staging/r8188eu/core/rtw_cmd.c b/drivers/staging/r8188eu/core/rtw_cmd.c
index b834fac41627b24f28576cc671825942fd46ff79..bdf0d0160070eaa5a738ad67e90597cbaa2688cf 100644 (file)
--- a/drivers/staging/r8188eu/core/rtw_cmd.c
+++ b/drivers/staging/r8188eu/core/rtw_cmd.c
@@ -23,7 +23,8 @@ static int _rtw_init_cmd_priv(struct cmd_priv *pcmdpriv)
 
        sema_init(&pcmdpriv->cmd_queue_sema, 0);
        /* sema_init(&(pcmdpriv->cmd_done_sema), 0); */
-       sema_init(&pcmdpriv->terminate_cmdthread_sema, 0);
+       init_completion(&pcmdpriv->start_cmd_thread);
+       init_completion(&pcmdpriv->stop_cmd_thread);
 
        rtw_init_queue(&pcmdpriv->cmd_queue);
 
@@ -246,7 +247,7 @@ int rtw_cmd_thread(void *context)
        pcmdbuf = pcmdpriv->cmd_buf;
 
        pcmdpriv->cmdthd_running = true;
-       up(&pcmdpriv->terminate_cmdthread_sema);
+       complete(&pcmdpriv->start_cmd_thread);
 
        while (1) {
                if (_rtw_down_sema(&pcmdpriv->cmd_queue_sema) == _FAIL)
@@ -327,7 +328,7 @@ post_process:
                rtw_free_cmd_obj(pcmd);
        } while (1);
 
-       up(&pcmdpriv->terminate_cmdthread_sema);
+       complete(&pcmdpriv->stop_cmd_thread);
 
        thread_exit();
 }

diff --git a/drivers/staging/r8188eu/include/rtw_cmd.h b/drivers/staging/r8188eu/include/rtw_cmd.h
index 83fbb922db2c32b7149e260542211da9d9ce83eb..b6266e3e2c4077926ba57b3030c53d6eddc8b44b 100644 (file)
--- a/drivers/staging/r8188eu/include/rtw_cmd.h
+++ b/drivers/staging/r8188eu/include/rtw_cmd.h
@@ -34,7 +34,8 @@ struct cmd_obj {
 
 struct cmd_priv {
        struct semaphore cmd_queue_sema;
-       struct semaphore terminate_cmdthread_sema;
+       struct completion start_cmd_thread;
+       struct completion stop_cmd_thread;
        struct __queue cmd_queue;
        u8      cmd_seq;
        u8      *cmd_buf;       /* shall be non-paged, and 4 bytes aligned */

diff --git a/drivers/staging/r8188eu/os_dep/os_intfs.c b/drivers/staging/r8188eu/os_dep/os_intfs.c
index e7964a048c996b7a3a63b3b767d9a8b6b1fbfab8..0bcea66f550b9a60384ce1328430b2124c62d0c1 100644 (file)
--- a/drivers/staging/r8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/r8188eu/os_dep/os_intfs.c
@@ -385,7 +385,8 @@ u32 rtw_start_drv_threads(struct adapter *padapter)
        if (IS_ERR(padapter->cmdThread))
                _status = _FAIL;
        else
-               _rtw_down_sema(&padapter->cmdpriv.terminate_cmdthread_sema); /* wait for cmd_thread to run */
+               /* wait for rtw_cmd_thread() to start running */
+               wait_for_completion(&padapter->cmdpriv.start_cmd_thread);
 
        return _status;
 }
@@ -395,7 +396,8 @@ void rtw_stop_drv_threads(struct adapter *padapter)
        /* Below is to termindate rtw_cmd_thread & event_thread... */
        up(&padapter->cmdpriv.cmd_queue_sema);
        if (padapter->cmdThread)
-               _rtw_down_sema(&padapter->cmdpriv.terminate_cmdthread_sema);
+               /* wait for rtw_cmd_thread() to stop running */
+               wait_for_completion(&padapter->cmdpriv.stop_cmd_thread);
 }
 
 static u8 rtw_init_default_value(struct adapter *padapter)