From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Larry Finger <Larry.Finger@lwfinger.net>,
Phillip Potter <phil@philpotter.co.uk>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Dan Carpenter <dan.carpenter@oracle.com>,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Cc: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Subject: [PATCH 1/3] staging: r8188eu: Use completions for signaling start and end kthread
Date: Sat, 16 Oct 2021 11:10:40 +0200 [thread overview]
Message-ID: <20211016091042.19614-2-fmdefrancesco@gmail.com> (raw)
In-Reply-To: <20211016091042.19614-1-fmdefrancesco@gmail.com>
rtw_cmd_thread() "up(s)" a semaphore twice, first to notify callers when
its execution has started and then to notify when it is about to end.
It makes the same semaphore go "up" twice in the same kthread. This
construct makes Smatch to warn of duplicate "up(s)".
This kthread 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
wait_for_completion*() because the interruptible / killable versions are
not necessary.
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_cmd.c | 7 ++++---
drivers/staging/r8188eu/include/rtw_cmd.h | 3 ++-
drivers/staging/r8188eu/os_dep/os_intfs.c | 6 ++++--
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/r8188eu/core/rtw_cmd.c b/drivers/staging/r8188eu/core/rtw_cmd.c
index e17332677daa..195390449502 100644
--- 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);
@@ -248,7 +249,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)
@@ -329,7 +330,7 @@ int rtw_cmd_thread(void *context)
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 83fbb922db2c..b6266e3e2c40 100644
--- 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 e7964a048c99..0bcea66f550b 100644
--- 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)
--
2.33.0
next prev parent reply other threads:[~2021-10-16 9:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-16 9:10 [PATCH 0/3] staging: r8188eu: use completions and clean rtw_cmd_thread() Fabio M. De Francesco
2021-10-16 9:10 ` Fabio M. De Francesco [this message]
2021-10-16 19:26 ` [PATCH 1/3] staging: r8188eu: Use completions for signaling start and end kthread Dan Carpenter
2021-10-16 19:32 ` Fabio M. De Francesco
2021-10-16 9:10 ` [PATCH 2/3] staging: r8188eu: Use completions for signaling enqueueing Fabio M. De Francesco
2021-10-16 9:10 ` [PATCH 3/3] staging: r8188eu: Remove redundant 'if' statement Fabio M. De Francesco
2021-10-16 9:59 ` Martin Kaiser
2021-10-16 15:27 ` [PATCH 0/3] staging: r8188eu: use completions and clean rtw_cmd_thread() Phillip Potter
2021-10-16 17:54 ` Fabio M. De Francesco
2021-10-17 9:48 ` Phillip Potter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211016091042.19614-2-fmdefrancesco@gmail.com \
--to=fmdefrancesco@gmail.com \
--cc=Larry.Finger@lwfinger.net \
--cc=dan.carpenter@oracle.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=phil@philpotter.co.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox