DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/3] eal/windows: fix use of incorrect thread ID
@ 2021-05-02  2:33 Dmitry Kozlyuk
  2021-05-02  2:33 ` [dpdk-dev] [PATCH 2/3] eal/windows: fix interrupt thread handle leakage Dmitry Kozlyuk
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Dmitry Kozlyuk @ 2021-05-02  2:33 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Kozlyuk, stable, Harman Kalra, Narcisa Ana Maria Vasile,
	Dmitry Malloy, Pallavi Kadam

Interrupt thread ID retained its value after interrupt thread finish.
Other interrupt routines could then operate on the wrong thread.
Clear interrupt thread ID before thread termination.

Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/eal/windows/eal_interrupts.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/eal/windows/eal_interrupts.c b/lib/eal/windows/eal_interrupts.c
index 1d4cf794df..9cde02b003 100644
--- a/lib/eal/windows/eal_interrupts.c
+++ b/lib/eal/windows/eal_interrupts.c
@@ -46,8 +46,11 @@ eal_intr_thread_main(LPVOID arg __rte_unused)
 			eal_intr_process(&events[i]);
 	}
 
+	intr_thread = 0;
+
 	CloseHandle(intr_iocp);
 	intr_iocp = NULL;
+
 	return NULL;
 }
 
-- 
2.29.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH 2/3] eal/windows: fix interrupt thread handle leakage
  2021-05-02  2:33 [dpdk-dev] [PATCH 1/3] eal/windows: fix use of incorrect thread ID Dmitry Kozlyuk
@ 2021-05-02  2:33 ` Dmitry Kozlyuk
  2021-05-13  1:15   ` Kadam, Pallavi
  2021-05-02  2:33 ` [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources Dmitry Kozlyuk
  2021-05-04  0:08 ` [dpdk-dev] [PATCH 1/3] eal/windows: fix use of incorrect thread ID Tyler Retzlaff
  2 siblings, 1 reply; 12+ messages in thread
From: Dmitry Kozlyuk @ 2021-05-02  2:33 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Kozlyuk, stable, Harman Kalra, Narcisa Ana Maria Vasile,
	Dmitry Malloy, Pallavi Kadam

Each time a work was scheduled in the interrupt thread,
usually an alarm, a handle was opened but not closed.

Opening a handle is a system call, which harms alarm precision.
Instead of opening and closing a handle each time, open it
when interrupt thread starts and close it when the thread finishes.

Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/eal/windows/eal_interrupts.c | 34 +++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/lib/eal/windows/eal_interrupts.c b/lib/eal/windows/eal_interrupts.c
index 9cde02b003..f24ed6e54e 100644
--- a/lib/eal/windows/eal_interrupts.c
+++ b/lib/eal/windows/eal_interrupts.c
@@ -10,6 +10,7 @@
 static pthread_t intr_thread;
 
 static HANDLE intr_iocp;
+static HANDLE intr_thread_handle;
 
 static void
 eal_intr_process(const OVERLAPPED_ENTRY *event)
@@ -17,9 +18,27 @@ eal_intr_process(const OVERLAPPED_ENTRY *event)
 	RTE_SET_USED(event);
 }
 
+static int
+eal_intr_thread_handle_init(void)
+{
+	DWORD thread_id = GetCurrentThreadId();
+
+	intr_thread_handle = OpenThread(THREAD_ALL_ACCESS, FALSE, thread_id);
+	if (intr_thread_handle == NULL) {
+		RTE_LOG_WIN32_ERR("OpenThread(%lu)", thread_id);
+		return -1;
+	}
+	return 0;
+}
+
 static void *
 eal_intr_thread_main(LPVOID arg __rte_unused)
 {
+	if (eal_intr_thread_handle_init() < 0) {
+		RTE_LOG(ERR, EAL, "Cannot open interrupt thread handle\n");
+		goto cleanup;
+	}
+
 	while (1) {
 		OVERLAPPED_ENTRY events[16];
 		ULONG event_count, i;
@@ -46,6 +65,10 @@ eal_intr_thread_main(LPVOID arg __rte_unused)
 			eal_intr_process(&events[i]);
 	}
 
+	CloseHandle(intr_thread_handle);
+	intr_thread_handle = NULL;
+
+cleanup:
 	intr_thread = 0;
 
 	CloseHandle(intr_iocp);
@@ -93,15 +116,8 @@ rte_intr_rx_ctl(__rte_unused struct rte_intr_handle *intr_handle,
 int
 eal_intr_thread_schedule(void (*func)(void *arg), void *arg)
 {
-	HANDLE handle;
-
-	handle = OpenThread(THREAD_ALL_ACCESS, FALSE, intr_thread);
-	if (handle == NULL) {
-		RTE_LOG_WIN32_ERR("OpenThread(%llu)", intr_thread);
-		return -ENOENT;
-	}
-
-	if (!QueueUserAPC((PAPCFUNC)(ULONG_PTR)func, handle, (ULONG_PTR)arg)) {
+	if (!QueueUserAPC((PAPCFUNC)(ULONG_PTR)func,
+			intr_thread_handle, (ULONG_PTR)arg)) {
 		RTE_LOG_WIN32_ERR("QueueUserAPC()");
 		return -EINVAL;
 	}
-- 
2.29.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources
  2021-05-02  2:33 [dpdk-dev] [PATCH 1/3] eal/windows: fix use of incorrect thread ID Dmitry Kozlyuk
  2021-05-02  2:33 ` [dpdk-dev] [PATCH 2/3] eal/windows: fix interrupt thread handle leakage Dmitry Kozlyuk
@ 2021-05-02  2:33 ` Dmitry Kozlyuk
  2021-05-11  7:41   ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
                     ` (2 more replies)
  2021-05-04  0:08 ` [dpdk-dev] [PATCH 1/3] eal/windows: fix use of incorrect thread ID Tyler Retzlaff
  2 siblings, 3 replies; 12+ messages in thread
From: Dmitry Kozlyuk @ 2021-05-02  2:33 UTC (permalink / raw)
  To: dev
  Cc: Dmitry Kozlyuk, stable, Narcisa Ana Maria Vasile, Dmitry Malloy,
	Pallavi Kadam, Harman Kalra

Interrupt manager in Windows EAL allocates on IOCP and starts
a control thread that runs indefinitely. At DPDK cleanup
this thread was not stopped and IOCP handle was not closed.

Gracefully stop interrupt-handling in rte_eal_cleanup().
The thread already closes IOCP handle before exiting.

Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
Cc: stable@dpdk.org

Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
---
 lib/eal/windows/eal.c            |  1 +
 lib/eal/windows/eal_interrupts.c | 26 ++++++++++++++++++++++++--
 lib/eal/windows/eal_windows.h    |  5 +++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
index 28c787c0b0..25afc42f8e 100644
--- a/lib/eal/windows/eal.c
+++ b/lib/eal/windows/eal.c
@@ -258,6 +258,7 @@ rte_eal_cleanup(void)
 {
 	struct internal_config *internal_conf =
 		eal_get_internal_configuration();
+	eal_intr_thread_cancel();
 	/* after this point, any DPDK pointers will become dangling */
 	rte_eal_memory_detach();
 	eal_cleanup_config(internal_conf);
diff --git a/lib/eal/windows/eal_interrupts.c b/lib/eal/windows/eal_interrupts.c
index f24ed6e54e..bb0585cb34 100644
--- a/lib/eal/windows/eal_interrupts.c
+++ b/lib/eal/windows/eal_interrupts.c
@@ -7,6 +7,8 @@
 #include "eal_private.h"
 #include "eal_windows.h"
 
+#define IOCP_KEY_SHUTDOWN UINT32_MAX
+
 static pthread_t intr_thread;
 
 static HANDLE intr_iocp;
@@ -34,12 +36,14 @@ eal_intr_thread_handle_init(void)
 static void *
 eal_intr_thread_main(LPVOID arg __rte_unused)
 {
+	bool finished = false;
+
 	if (eal_intr_thread_handle_init() < 0) {
 		RTE_LOG(ERR, EAL, "Cannot open interrupt thread handle\n");
 		goto cleanup;
 	}
 
-	while (1) {
+	while (!finished) {
 		OVERLAPPED_ENTRY events[16];
 		ULONG event_count, i;
 		BOOL result;
@@ -61,8 +65,13 @@ eal_intr_thread_main(LPVOID arg __rte_unused)
 			continue;
 		}
 
-		for (i = 0; i < event_count; i++)
+		for (i = 0; i < event_count; i++) {
+			if (events[i].lpCompletionKey == IOCP_KEY_SHUTDOWN) {
+				finished = true;
+				break;
+			}
 			eal_intr_process(&events[i]);
+		}
 	}
 
 	CloseHandle(intr_thread_handle);
@@ -125,6 +134,19 @@ eal_intr_thread_schedule(void (*func)(void *arg), void *arg)
 	return 0;
 }
 
+void
+eal_intr_thread_cancel(void)
+{
+	if (!PostQueuedCompletionStatus(
+			intr_iocp, 0, IOCP_KEY_SHUTDOWN, NULL)) {
+		RTE_LOG_WIN32_ERR("PostQueuedCompletionStatus()");
+		RTE_LOG(ERR, EAL, "Cannot cancel interrupt thread\n");
+		return;
+	}
+
+	WaitForSingleObject(intr_thread_handle, INFINITE);
+}
+
 int
 rte_intr_callback_register(
 	__rte_unused const struct rte_intr_handle *intr_handle,
diff --git a/lib/eal/windows/eal_windows.h b/lib/eal/windows/eal_windows.h
index 478accc1b9..7cc811485d 100644
--- a/lib/eal/windows/eal_windows.h
+++ b/lib/eal/windows/eal_windows.h
@@ -67,6 +67,11 @@ unsigned int eal_socket_numa_node(unsigned int socket_id);
  */
 int eal_intr_thread_schedule(void (*func)(void *arg), void *arg);
 
+/**
+ * Request interrupt thread to stop and wait its termination.
+ */
+void eal_intr_thread_cancel(void);
+
 /**
  * Open virt2phys driver interface device.
  *
-- 
2.29.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 1/3] eal/windows: fix use of incorrect thread ID
  2021-05-02  2:33 [dpdk-dev] [PATCH 1/3] eal/windows: fix use of incorrect thread ID Dmitry Kozlyuk
  2021-05-02  2:33 ` [dpdk-dev] [PATCH 2/3] eal/windows: fix interrupt thread handle leakage Dmitry Kozlyuk
  2021-05-02  2:33 ` [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources Dmitry Kozlyuk
@ 2021-05-04  0:08 ` Tyler Retzlaff
  2 siblings, 0 replies; 12+ messages in thread
From: Tyler Retzlaff @ 2021-05-04  0:08 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, stable, Harman Kalra, Narcisa Ana Maria Vasile,
	Dmitry Malloy, Pallavi Kadam

On Sun, May 02, 2021 at 05:33:31AM +0300, Dmitry Kozlyuk wrote:
> Interrupt thread ID retained its value after interrupt thread finish.
> Other interrupt routines could then operate on the wrong thread.
> Clear interrupt thread ID before thread termination.
> 
> Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
> Cc: stable@dpdk.org

Acked-by: Tyler Retzlaff <linux.microsoft.com>

> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  lib/eal/windows/eal_interrupts.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/eal/windows/eal_interrupts.c b/lib/eal/windows/eal_interrupts.c
> index 1d4cf794df..9cde02b003 100644
> --- a/lib/eal/windows/eal_interrupts.c
> +++ b/lib/eal/windows/eal_interrupts.c
> @@ -46,8 +46,11 @@ eal_intr_thread_main(LPVOID arg __rte_unused)
>  			eal_intr_process(&events[i]);
>  	}
>  
> +	intr_thread = 0;
> +
>  	CloseHandle(intr_iocp);
>  	intr_iocp = NULL;
> +
>  	return NULL;
>  }
>  
> -- 
> 2.29.3

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 3/3] eal/windows: cleanup interrupt resources
  2021-05-02  2:33 ` [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources Dmitry Kozlyuk
@ 2021-05-11  7:41   ` Thomas Monjalon
  2021-05-11  7:59     ` Dmitry Kozlyuk
  2021-05-11 17:24   ` [dpdk-dev] " Ranjit Menon
  2021-05-28 17:33   ` Jie Zhou
  2 siblings, 1 reply; 12+ messages in thread
From: Thomas Monjalon @ 2021-05-11  7:41 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, Dmitry Kozlyuk, stable, Narcisa Ana Maria Vasile,
	Dmitry Malloy, Pallavi Kadam, Harman Kalra, Harini Ramakrishnan,
	ranjit.menon

02/05/2021 04:33, Dmitry Kozlyuk:
> Interrupt manager in Windows EAL allocates on IOCP and starts
> a control thread that runs indefinitely. At DPDK cleanup
> this thread was not stopped and IOCP handle was not closed.
> 
> Gracefully stop interrupt-handling in rte_eal_cleanup().
> The thread already closes IOCP handle before exiting.
> 
> Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>  lib/eal/windows/eal.c            |  1 +
>  lib/eal/windows/eal_interrupts.c | 26 ++++++++++++++++++++++++--
>  lib/eal/windows/eal_windows.h    |  5 +++++
>  3 files changed, 30 insertions(+), 2 deletions(-)

It seems nobody reviewed.
To be on the safe side, I'll merge this series after DPDK 21.05 is released.
Or am I missing any critical issue?




^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 3/3] eal/windows: cleanup interrupt resources
  2021-05-11  7:41   ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
@ 2021-05-11  7:59     ` Dmitry Kozlyuk
  2021-05-11 17:21       ` Ranjit Menon
  0 siblings, 1 reply; 12+ messages in thread
From: Dmitry Kozlyuk @ 2021-05-11  7:59 UTC (permalink / raw)
  To: Thomas Monjalon
  Cc: dev, stable, Narcisa Ana Maria Vasile, Dmitry Malloy,
	Pallavi Kadam, Harman Kalra, Harini Ramakrishnan, ranjit.menon

2021-05-11 09:41 (UTC+0200), Thomas Monjalon:
> 02/05/2021 04:33, Dmitry Kozlyuk:
> > Interrupt manager in Windows EAL allocates on IOCP and starts
> > a control thread that runs indefinitely. At DPDK cleanup
> > this thread was not stopped and IOCP handle was not closed.
> > 
> > Gracefully stop interrupt-handling in rte_eal_cleanup().
> > The thread already closes IOCP handle before exiting.
> > 
> > Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> > ---
> >  lib/eal/windows/eal.c            |  1 +
> >  lib/eal/windows/eal_interrupts.c | 26 ++++++++++++++++++++++++--
> >  lib/eal/windows/eal_windows.h    |  5 +++++
> >  3 files changed, 30 insertions(+), 2 deletions(-)  
> 
> It seems nobody reviewed.
> To be on the safe side, I'll merge this series after DPDK 21.05 is released.
> Or am I missing any critical issue?

IIRC Windows DPDK is not shipped anywhere yet, so the fix can be postponed.

Without fix in 2/3 rte_eal_alarm_set() will start failing after some
thousands of calls (i40e calls every 50 ms, mlx5 call every 1 sec or less).
For mlx5 it seems to break flow counters (mlx5_flow_query_alarm function).

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 3/3] eal/windows: cleanup interrupt resources
  2021-05-11  7:59     ` Dmitry Kozlyuk
@ 2021-05-11 17:21       ` Ranjit Menon
  2021-05-12 14:56         ` Thomas Monjalon
  0 siblings, 1 reply; 12+ messages in thread
From: Ranjit Menon @ 2021-05-11 17:21 UTC (permalink / raw)
  To: Dmitry Kozlyuk, Thomas Monjalon
  Cc: dev, stable, Narcisa Ana Maria Vasile, Dmitry Malloy,
	Pallavi Kadam, Harman Kalra, Harini Ramakrishnan


On 5/11/2021 12:59 AM, Dmitry Kozlyuk wrote:
> 2021-05-11 09:41 (UTC+0200), Thomas Monjalon:
>> 02/05/2021 04:33, Dmitry Kozlyuk:
>>> Interrupt manager in Windows EAL allocates on IOCP and starts
>>> a control thread that runs indefinitely. At DPDK cleanup
>>> this thread was not stopped and IOCP handle was not closed.
>>>
>>> Gracefully stop interrupt-handling in rte_eal_cleanup().
>>> The thread already closes IOCP handle before exiting.
>>>
>>> Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
>>> ---
>>>   lib/eal/windows/eal.c            |  1 +
>>>   lib/eal/windows/eal_interrupts.c | 26 ++++++++++++++++++++++++--
>>>   lib/eal/windows/eal_windows.h    |  5 +++++
>>>   3 files changed, 30 insertions(+), 2 deletions(-)
>> It seems nobody reviewed.
>> To be on the safe side, I'll merge this series after DPDK 21.05 is released.
>> Or am I missing any critical issue?
> IIRC Windows DPDK is not shipped anywhere yet, so the fix can be postponed.
>
> Without fix in 2/3 rte_eal_alarm_set() will start failing after some
> thousands of calls (i40e calls every 50 ms, mlx5 call every 1 sec or less).
> For mlx5 it seems to break flow counters (mlx5_flow_query_alarm function).

It appears that Tyler reviewed and ack-ed this. I'll add my ACK too. If 
we can get this in to 21.05, it would be great.

ranjit m.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources
  2021-05-02  2:33 ` [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources Dmitry Kozlyuk
  2021-05-11  7:41   ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
@ 2021-05-11 17:24   ` Ranjit Menon
  2021-05-28 17:33   ` Jie Zhou
  2 siblings, 0 replies; 12+ messages in thread
From: Ranjit Menon @ 2021-05-11 17:24 UTC (permalink / raw)
  To: Dmitry Kozlyuk, dev
  Cc: stable, Narcisa Ana Maria Vasile, Dmitry Malloy, Pallavi Kadam,
	Harman Kalra


On 5/1/2021 7:33 PM, Dmitry Kozlyuk wrote:
> Interrupt manager in Windows EAL allocates on IOCP and starts
> a control thread that runs indefinitely. At DPDK cleanup
> this thread was not stopped and IOCP handle was not closed.
>
> Gracefully stop interrupt-handling in rte_eal_cleanup().
> The thread already closes IOCP handle before exiting.
>
> Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
> Cc: stable@dpdk.org
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---
>   lib/eal/windows/eal.c            |  1 +
>   lib/eal/windows/eal_interrupts.c | 26 ++++++++++++++++++++++++--
>   lib/eal/windows/eal_windows.h    |  5 +++++
>   3 files changed, 30 insertions(+), 2 deletions(-)
>
> diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
> index 28c787c0b0..25afc42f8e 100644
> --- a/lib/eal/windows/eal.c
> +++ b/lib/eal/windows/eal.c
> @@ -258,6 +258,7 @@ rte_eal_cleanup(void)
>   {
>   	struct internal_config *internal_conf =
>   		eal_get_internal_configuration();
> +	eal_intr_thread_cancel();
A blank line above this line would be nice.
>   	/* after this point, any DPDK pointers will become dangling */
>   	rte_eal_memory_detach();
>   	eal_cleanup_config(internal_conf);
> diff --git a/lib/eal/windows/eal_interrupts.c b/lib/eal/windows/eal_interrupts.c
> index f24ed6e54e..bb0585cb34 100644
> --- a/lib/eal/windows/eal_interrupts.c
> +++ b/lib/eal/windows/eal_interrupts.c
> @@ -7,6 +7,8 @@
>   #include "eal_private.h"
>   #include "eal_windows.h"
>   
> +#define IOCP_KEY_SHUTDOWN UINT32_MAX
> +
>   static pthread_t intr_thread;
>   
>   static HANDLE intr_iocp;
> @@ -34,12 +36,14 @@ eal_intr_thread_handle_init(void)
>   static void *
>   eal_intr_thread_main(LPVOID arg __rte_unused)
>   {
> +	bool finished = false;
> +
>   	if (eal_intr_thread_handle_init() < 0) {
>   		RTE_LOG(ERR, EAL, "Cannot open interrupt thread handle\n");
>   		goto cleanup;
>   	}
>   
> -	while (1) {
> +	while (!finished) {
>   		OVERLAPPED_ENTRY events[16];
>   		ULONG event_count, i;
>   		BOOL result;
> @@ -61,8 +65,13 @@ eal_intr_thread_main(LPVOID arg __rte_unused)
>   			continue;
>   		}
>   
> -		for (i = 0; i < event_count; i++)
> +		for (i = 0; i < event_count; i++) {
> +			if (events[i].lpCompletionKey == IOCP_KEY_SHUTDOWN) {
> +				finished = true;
> +				break;
> +			}
>   			eal_intr_process(&events[i]);
> +		}
>   	}
>   
>   	CloseHandle(intr_thread_handle);
> @@ -125,6 +134,19 @@ eal_intr_thread_schedule(void (*func)(void *arg), void *arg)
>   	return 0;
>   }
>   
> +void
> +eal_intr_thread_cancel(void)
> +{
> +	if (!PostQueuedCompletionStatus(
> +			intr_iocp, 0, IOCP_KEY_SHUTDOWN, NULL)) {
> +		RTE_LOG_WIN32_ERR("PostQueuedCompletionStatus()");
> +		RTE_LOG(ERR, EAL, "Cannot cancel interrupt thread\n");
> +		return;
> +	}
> +
> +	WaitForSingleObject(intr_thread_handle, INFINITE);
> +}
> +
>   int
>   rte_intr_callback_register(
>   	__rte_unused const struct rte_intr_handle *intr_handle,
> diff --git a/lib/eal/windows/eal_windows.h b/lib/eal/windows/eal_windows.h
> index 478accc1b9..7cc811485d 100644
> --- a/lib/eal/windows/eal_windows.h
> +++ b/lib/eal/windows/eal_windows.h
> @@ -67,6 +67,11 @@ unsigned int eal_socket_numa_node(unsigned int socket_id);
>    */
>   int eal_intr_thread_schedule(void (*func)(void *arg), void *arg);
>   
> +/**
> + * Request interrupt thread to stop and wait its termination.
> + */
> +void eal_intr_thread_cancel(void);
> +
>   /**
>    * Open virt2phys driver interface device.
>    *

Other than nit above,

Acked-by: Ranjit Menon <ranjit.menon@intel.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH 3/3] eal/windows: cleanup interrupt resources
  2021-05-11 17:21       ` Ranjit Menon
@ 2021-05-12 14:56         ` Thomas Monjalon
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2021-05-12 14:56 UTC (permalink / raw)
  To: Dmitry Kozlyuk, Ranjit Menon
  Cc: dev, stable, Narcisa Ana Maria Vasile, Dmitry Malloy,
	Pallavi Kadam, Harman Kalra, Harini Ramakrishnan, talshn,
	Tyler Retzlaff

11/05/2021 19:21, Ranjit Menon:
> On 5/11/2021 12:59 AM, Dmitry Kozlyuk wrote:
> > 2021-05-11 09:41 (UTC+0200), Thomas Monjalon:
> >> 02/05/2021 04:33, Dmitry Kozlyuk:
> >>> Interrupt manager in Windows EAL allocates on IOCP and starts
> >>> a control thread that runs indefinitely. At DPDK cleanup
> >>> this thread was not stopped and IOCP handle was not closed.
> >>>
> >>> Gracefully stop interrupt-handling in rte_eal_cleanup().
> >>> The thread already closes IOCP handle before exiting.
> >>>
> >>> Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> >>> ---
> >>>   lib/eal/windows/eal.c            |  1 +
> >>>   lib/eal/windows/eal_interrupts.c | 26 ++++++++++++++++++++++++--
> >>>   lib/eal/windows/eal_windows.h    |  5 +++++
> >>>   3 files changed, 30 insertions(+), 2 deletions(-)
> >> It seems nobody reviewed.
> >> To be on the safe side, I'll merge this series after DPDK 21.05 is released.
> >> Or am I missing any critical issue?
> > IIRC Windows DPDK is not shipped anywhere yet, so the fix can be postponed.
> >
> > Without fix in 2/3 rte_eal_alarm_set() will start failing after some
> > thousands of calls (i40e calls every 50 ms, mlx5 call every 1 sec or less).
> > For mlx5 it seems to break flow counters (mlx5_flow_query_alarm function).
> 
> It appears that Tyler reviewed and ack-ed this. I'll add my ACK too. If 
> we can get this in to 21.05, it would be great.

Tyler acked only the patch 1.

It would be good to have tests with mlx5 and i40e for the patch 2.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 2/3] eal/windows: fix interrupt thread handle leakage
  2021-05-02  2:33 ` [dpdk-dev] [PATCH 2/3] eal/windows: fix interrupt thread handle leakage Dmitry Kozlyuk
@ 2021-05-13  1:15   ` Kadam, Pallavi
  0 siblings, 0 replies; 12+ messages in thread
From: Kadam, Pallavi @ 2021-05-13  1:15 UTC (permalink / raw)
  To: Dmitry Kozlyuk, dev
  Cc: stable, Harman Kalra, Narcisa Ana Maria Vasile, Dmitry Malloy


On 5/1/2021 7:33 PM, Dmitry Kozlyuk wrote:
> Each time a work was scheduled in the interrupt thread,
> usually an alarm, a handle was opened but not closed.
>
> Opening a handle is a system call, which harms alarm precision.
> Instead of opening and closing a handle each time, open it
> when interrupt thread starts and close it when the thread finishes.
>
> Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
> Cc: stable@dpdk.org
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> ---

Works with i40e.

Tested-by: Pallavi Kadam <pallavi.kadam@intel.com>

>   

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources
  2021-05-02  2:33 ` [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources Dmitry Kozlyuk
  2021-05-11  7:41   ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
  2021-05-11 17:24   ` [dpdk-dev] " Ranjit Menon
@ 2021-05-28 17:33   ` Jie Zhou
  2021-06-23  7:08     ` Thomas Monjalon
  2 siblings, 1 reply; 12+ messages in thread
From: Jie Zhou @ 2021-05-28 17:33 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, stable, Narcisa Ana Maria Vasile, Dmitry Malloy,
	Pallavi Kadam, Harman Kalra

On Sun, May 02, 2021 at 05:33:33AM +0300, Dmitry Kozlyuk wrote:
> Interrupt manager in Windows EAL allocates on IOCP and starts
> a control thread that runs indefinitely. At DPDK cleanup
> this thread was not stopped and IOCP handle was not closed.
> 
> Gracefully stop interrupt-handling in rte_eal_cleanup().
> The thread already closes IOCP handle before exiting.
> 
> Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>

Acked-by: Jie Zhou <jizh@microsoft.com>
Tested-by: Jie Zhou <jizh@microsoft.com>

> ---
>  lib/eal/windows/eal.c            |  1 +
>  lib/eal/windows/eal_interrupts.c | 26 ++++++++++++++++++++++++--
>  lib/eal/windows/eal_windows.h    |  5 +++++
>  3 files changed, 30 insertions(+), 2 deletions(-)

Enabled a subset of Unit test on Windows and when running alarm_autotest, system hang at rte_eal_alarm_set. After applying this patch set, no repro any more. Also system hang at pmd_perf_autotest and no repro with the patch. It is with Intel i40e.
 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources
  2021-05-28 17:33   ` Jie Zhou
@ 2021-06-23  7:08     ` Thomas Monjalon
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Monjalon @ 2021-06-23  7:08 UTC (permalink / raw)
  To: Dmitry Kozlyuk
  Cc: dev, stable, Narcisa Ana Maria Vasile, Dmitry Malloy,
	Pallavi Kadam, Harman Kalra, Jie Zhou

28/05/2021 19:33, Jie Zhou:
> On Sun, May 02, 2021 at 05:33:33AM +0300, Dmitry Kozlyuk wrote:
> > Interrupt manager in Windows EAL allocates on IOCP and starts
> > a control thread that runs indefinitely. At DPDK cleanup
> > this thread was not stopped and IOCP handle was not closed.
> > 
> > Gracefully stop interrupt-handling in rte_eal_cleanup().
> > The thread already closes IOCP handle before exiting.
> > 
> > Fixes: 5c016fc0205a ("eal/windows: add interrupt thread skeleton")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
> 
> Acked-by: Jie Zhou <jizh@microsoft.com>
> Tested-by: Jie Zhou <jizh@microsoft.com>

Series applied, thanks.




^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-06-23  7:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-02  2:33 [dpdk-dev] [PATCH 1/3] eal/windows: fix use of incorrect thread ID Dmitry Kozlyuk
2021-05-02  2:33 ` [dpdk-dev] [PATCH 2/3] eal/windows: fix interrupt thread handle leakage Dmitry Kozlyuk
2021-05-13  1:15   ` Kadam, Pallavi
2021-05-02  2:33 ` [dpdk-dev] [PATCH 3/3] eal/windows: cleanup interrupt resources Dmitry Kozlyuk
2021-05-11  7:41   ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
2021-05-11  7:59     ` Dmitry Kozlyuk
2021-05-11 17:21       ` Ranjit Menon
2021-05-12 14:56         ` Thomas Monjalon
2021-05-11 17:24   ` [dpdk-dev] " Ranjit Menon
2021-05-28 17:33   ` Jie Zhou
2021-06-23  7:08     ` Thomas Monjalon
2021-05-04  0:08 ` [dpdk-dev] [PATCH 1/3] eal/windows: fix use of incorrect thread ID Tyler Retzlaff

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).