From: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
To: dev@dpdk.org
Cc: "Kadam, Pallavi" <pallavi.kadam@intel.com>,
Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>,
Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>,
Dmitry Malloy <dmitrym@microsoft.com>
Subject: [dpdk-dev] [PATCH] eal/windows: fix deadlock when setting alarm
Date: Fri, 30 Oct 2020 21:31:13 +0300 [thread overview]
Message-ID: <20201030183113.17829-1-dmitry.kozliuk@gmail.com> (raw)
Windows alarms are bot harmed and executed from the interrupt thread.
rte_eal_alarm_set() dispatched code to arm an alarm to that thread and
waited for its completion via a spinlock. However, if called from alarm
callback (i.e. from the interrupt thread), this caused a deadlock,
because arming could not be run until its dispatcher exits, but it could
only exit after arming has finished.
Call arming code directly when running in the interrupt thread.
Fixes: f4cbdbc7fbd2 ("eal/windows: implement alarm API")
Reported-by: Pallavi Kadam <pallavi.kadam@intel.com>
Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
lib/librte_eal/windows/eal_alarm.c | 68 ++++++++++++++++++++----------
1 file changed, 46 insertions(+), 22 deletions(-)
diff --git a/lib/librte_eal/windows/eal_alarm.c b/lib/librte_eal/windows/eal_alarm.c
index 3b262793a..f5bf88715 100644
--- a/lib/librte_eal/windows/eal_alarm.c
+++ b/lib/librte_eal/windows/eal_alarm.c
@@ -30,7 +30,7 @@ static LIST_HEAD(alarm_list, alarm_entry) alarm_list = LIST_HEAD_INITIALIZER();
static rte_spinlock_t alarm_lock = RTE_SPINLOCK_INITIALIZER;
-static int intr_thread_exec(void (*func)(void *arg), void *arg);
+static int intr_thread_exec_sync(void (*func)(void *arg), void *arg);
static void
alarm_remove_unsafe(struct alarm_entry *ap)
@@ -57,6 +57,18 @@ alarm_callback(void *arg, DWORD low __rte_unused, DWORD high __rte_unused)
rte_spinlock_unlock(&alarm_lock);
}
+static int
+alarm_set(struct alarm_entry *entry, LARGE_INTEGER deadline)
+{
+ BOOL ret = SetWaitableTimer(
+ entry->timer, &deadline, 0, alarm_callback, entry, FALSE);
+ if (!ret) {
+ RTE_LOG_WIN32_ERR("SetWaitableTimer");
+ return -1;
+ }
+ return 0;
+}
+
struct alarm_task {
struct alarm_entry *entry;
LARGE_INTEGER deadline;
@@ -64,14 +76,10 @@ struct alarm_task {
};
static void
-alarm_set(void *arg)
+alarm_task_exec(void *arg)
{
struct alarm_task *task = arg;
-
- BOOL ret = SetWaitableTimer(
- task->entry->timer, &task->deadline,
- 0, alarm_callback, task->entry, FALSE);
- task->ret = ret ? 0 : (-1);
+ task->ret = alarm_set(task->entry, task->deadline);
}
int
@@ -80,14 +88,14 @@ rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg)
struct alarm_entry *ap;
HANDLE timer;
FILETIME ft;
- struct alarm_task task;
+ LARGE_INTEGER deadline;
int ret;
/* Calculate deadline ASAP, unit of measure = 100ns. */
GetSystemTimePreciseAsFileTime(&ft);
- task.deadline.LowPart = ft.dwLowDateTime;
- task.deadline.HighPart = ft.dwHighDateTime;
- task.deadline.QuadPart += 10 * us;
+ deadline.LowPart = ft.dwLowDateTime;
+ deadline.HighPart = ft.dwHighDateTime;
+ deadline.QuadPart += 10 * us;
ap = calloc(1, sizeof(*ap));
if (ap == NULL) {
@@ -106,21 +114,37 @@ rte_eal_alarm_set(uint64_t us, rte_eal_alarm_callback cb_fn, void *cb_arg)
ap->timer = timer;
ap->cb_fn = cb_fn;
ap->cb_arg = cb_arg;
- task.entry = ap;
/* Waitable timer must be set in the same thread that will
* do an alertable wait for the alarm to trigger, that is,
- * in the interrupt thread. Setting can fail, so do it synchronously.
+ * in the interrupt thread.
*/
- ret = intr_thread_exec(alarm_set, &task);
- if (ret < 0) {
- RTE_LOG(ERR, EAL, "Cannot setup alarm in interrupt thread\n");
- goto fail;
- }
+ if (rte_thread_is_intr()) {
+ /* Directly schedule callback execution. */
+ ret = alarm_set(ap, deadline);
+ if (ret < 0) {
+ RTE_LOG(ERR, EAL, "Cannot setup alarm\n");
+ goto fail;
+ }
+ } else {
+ /* Dispatch a task to set alarm into the interrupt thread.
+ * Execute it synchronously, because it can fail.
+ */
+ struct alarm_task task = {
+ .entry = ap,
+ .deadline = deadline,
+ };
+
+ ret = intr_thread_exec_sync(alarm_task_exec, &task);
+ if (ret < 0) {
+ RTE_LOG(ERR, EAL, "Cannot setup alarm in interrupt thread\n");
+ goto fail;
+ }
- ret = task.ret;
- if (ret < 0)
- goto fail;
+ ret = task.ret;
+ if (ret < 0)
+ goto fail;
+ }
rte_spinlock_lock(&alarm_lock);
LIST_INSERT_HEAD(&alarm_list, ap, next);
@@ -196,7 +220,7 @@ intr_thread_entry(void *arg)
}
static int
-intr_thread_exec(void (*func)(void *arg), void *arg)
+intr_thread_exec_sync(void (*func)(void *arg), void *arg)
{
struct intr_task task;
int ret;
--
2.28.0
next reply other threads:[~2020-10-30 18:44 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-30 18:31 Dmitry Kozlyuk [this message]
2020-10-30 18:42 ` [dpdk-dev] [PATCH v2] " Dmitry Kozlyuk
2020-10-30 19:25 ` Kadam, Pallavi
2020-11-03 20:32 ` Thomas Monjalon
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=20201030183113.17829-1-dmitry.kozliuk@gmail.com \
--to=dmitry.kozliuk@gmail.com \
--cc=dev@dpdk.org \
--cc=dmitrym@microsoft.com \
--cc=navasile@linux.microsoft.com \
--cc=pallavi.kadam@intel.com \
/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;
as well as URLs for NNTP newsgroup(s).