From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 0E24EA04E6;
	Fri, 30 Oct 2020 19:44:14 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id CB2F566DB;
	Fri, 30 Oct 2020 19:31:22 +0100 (CET)
Received: from mail-lj1-f193.google.com (mail-lj1-f193.google.com
 [209.85.208.193]) by dpdk.org (Postfix) with ESMTP id 7370266DB
 for <dev@dpdk.org>; Fri, 30 Oct 2020 19:31:21 +0100 (CET)
Received: by mail-lj1-f193.google.com with SMTP id i2so7959599ljg.4
 for <dev@dpdk.org>; Fri, 30 Oct 2020 11:31:21 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=from:to:cc:subject:date:message-id:mime-version
 :content-transfer-encoding;
 bh=eTgVtz180maEXJL2VazxeRWuFwcrZbcwgb7/2TxwMTA=;
 b=pSN2BNc/3Si07/w7Yami50PjXPJTSBqNoWTqR9dGptPuVwEX6xmB9+Ktmw84MqcNi5
 MXKRA8XkoxUAc7CQJMhLOuOhP2DhF12BEnDW0T4NQxhgTT2Izo62yfHybNLz5L0p63Yd
 ijlgrVVfn3gn33speXbTseEkOE4S1bqvXpPdRem+yowq3dHau2pRZ6AD6uybFt5Rg2cF
 PYrXEzxE+GX6PTUEKudshTwVGqGUWGPP/LdpBjIUA5KkUlQX+ADCWs9Q5mbt1CMfYKzH
 KAor08Ygc9u4OXZh3e4CvjnKRt2njUcM/C8lc8pghFUVaH9DzIHSqIsKyve62eCiKU62
 KhEA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version
 :content-transfer-encoding;
 bh=eTgVtz180maEXJL2VazxeRWuFwcrZbcwgb7/2TxwMTA=;
 b=mzHXExeFBwwXlJWcrNM71AIZlzHIPjwqOawCcZiLq34jWKxdT6bvMfk/e+HDckPcmR
 WUsaNkBXU49QGWHMBT4NqU1nwbuSztJTlibNlx2KbkQbwwI8iXkmJTWatoNg6sGC8eEo
 rY+BEnVfHN318vPJi7SJWi+zjRK+WJjKPq4boIcvZ1Og0aDHURR9BEM7B2CT7enXmXGb
 Xh7bGsWLXwQDXK2p5MA5W36AwasFBYs2Mqa8qbL6JjpXLjlhIrPcE7hQlsT1w05tCkSl
 EFcWkVGStG57X3ACYrYr3E8LM+WABWQxuNIUNLbPCAB1FQlUeCAfRZJZAlPM9fxWT+cs
 dMDQ==
X-Gm-Message-State: AOAM532x9aGtMH8nzGP1oGpHOuiKTvimgfuEPx605VT2Sn9GV9hCdbvG
 g+Yhv+LLaCmwfXM/MpiECjFpKWQ/RSs94Mzu
X-Google-Smtp-Source: ABdhPJwkaKqEDqqoHsNQFDreQFDyijdTMEey2gsi60aTYwNTlzdTz9vq5hLtpaUUw7irQ8ZT7yEshg==
X-Received: by 2002:a2e:5750:: with SMTP id r16mr1569792ljd.31.1604082679394; 
 Fri, 30 Oct 2020 11:31:19 -0700 (PDT)
Received: from localhost.localdomain (broadband-37-110-65-23.ip.moscow.rt.ru.
 [37.110.65.23])
 by smtp.gmail.com with ESMTPSA id r5sm755741ljm.77.2020.10.30.11.31.18
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Fri, 30 Oct 2020 11:31:18 -0700 (PDT)
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>
Date: Fri, 30 Oct 2020 21:31:13 +0300
Message-Id: <20201030183113.17829-1-dmitry.kozliuk@gmail.com>
X-Mailer: git-send-email 2.28.0
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [dpdk-dev] [PATCH] eal/windows: fix deadlock when setting alarm
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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