From patchwork Fri Oct 30 18:31:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Kozlyuk X-Patchwork-Id: 83040 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.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 ; Fri, 30 Oct 2020 19:31:21 +0100 (CET) Received: by mail-lj1-f193.google.com with SMTP id i2so7959599ljg.4 for ; 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 To: dev@dpdk.org Cc: "Kadam, Pallavi" , Dmitry Kozlyuk , Narcisa Ana Maria Vasile , Dmitry Malloy 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 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 Signed-off-by: Dmitry Kozlyuk --- 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;