DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [dpdk-dev, 1/3] rte_interrupts: add rte_eal_intr_exit to shut down IRQ thread
@ 2020-08-13 22:28 Bly, Mike
  0 siblings, 0 replies; 7+ messages in thread
From: Bly, Mike @ 2020-08-13 22:28 UTC (permalink / raw)
  To: cunming.liang; +Cc: dev, mhall, thomas.monjalon

Has anyone created a dev-ticket to run this discussion to ground? I see below thread went stale in 2016...



Is there a "better approach" to integrating rte_eal_intr_exit() support/concepts into our applications?



-Mike



From: "Liang, Cunming" <cunming.liang@intel.com>

To: Thomas Monjalon <thomas.monjalon@6wind.com>

Cc: Matthew Hall <mhall@mhcomputing.net>, "dev@dpdk.org" <dev@dpdk.org>

Subject: Re: [dpdk-dev] [dpdk-dev, 1/3] rte_interrupts: add rte_eal_intr_exit to shut down IRQ thread<http://inbox.dpdk.org/dev/D0158A423229094DA7ABF71CF2FA0DA31554689D@shsmsx102.ccr.corp.intel.com/#r>

Date: Mon, 11 Jul 2016 04:07:29 +0000

Message-ID: <D0158A423229094DA7ABF71CF2FA0DA31554689D@shsmsx102.ccr.corp.intel.com> (raw<http://inbox.dpdk.org/dev/D0158A423229094DA7ABF71CF2FA0DA31554689D@shsmsx102.ccr.corp.intel.com/raw>)

In-Reply-To: <7816883.NtLY7W8fN8@xps13<http://inbox.dpdk.org/dev/7816883.NtLY7W8fN8@xps13/>>



Hi Thomas,



Base on the previous conversation, at least it requires v2 to reword some comments.



> > >> 2. I propose to add addition comments on rte_epoll_wait() API

> > >> description. For any signal, it causes an error return, user needs

> > >> to handle.

> > > Agreed.



In addition, one conversion is not close.



> > >> the default termination handler

> > > I am not so experienced with this "default termination handler". Can

> someone

> > > clarify what it is so I could comment better about it?

> > For example, you're handling SIGINT. After finishing your necessary app

> > cleanup, then 'signal(SIGINT, SIG_DFL); raise(SIGINT);'.

> > The default signal handler can terminate the interrupt thread.



> > >> 1. Can you explain and add patch comments why default signal handler

> > >> is not good enough to terminate app.

> > > Yes if someone call tell me more about what it is so I can check it.



Thanks,

Cunming



> -----Original Message-----

> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]

> Sent: Saturday, July 09, 2016 1:36 AM

> To: Liang, Cunming <cunming.liang@intel.com>

> Cc: Matthew Hall <mhall@mhcomputing.net>; dev@dpdk.org

> Subject: Re: [dpdk-dev,1/3] rte_interrupts: add rte_eal_intr_exit to shut down

> IRQ thread

>

> Cunming, what is the status of this patchset, please?

>

> 2016-03-23 11:24, Liang, Cunming:

> > Hi Mattew,

> >

> > Thank you for your time.

> >

> > On 3/22/2016 3:39 PM, Matthew Hall wrote:

> > > On Mon, Mar 21, 2016 at 03:58:44PM +0800, Liang, Cunming wrote:

> > >> the default termination handler

> > > I am not so experienced with this "default termination handler". Can

> someone

> > > clarify what it is so I could comment better about it?

> > For example, you're handling SIGINT. After finishing your necessary app

> > cleanup, then 'signal(SIGINT, SIG_DFL); raise(SIGINT);'.

> > The default signal handler can terminate the interrupt thread.

> >

> > >

> > >> If EINTR is caused by some non-term purpose signals, are you going

> > >> to exit the interrupt thread any way?

> > > We should discuss what makes sense here. I'm just trying to get some things

> > > working and finding EINTR was getting eaten and causing infinite looping.

> > SIGINT/SIGTERM causes EINTR return, while SIGUSR1 also can cause the

> > EINTR return. For the dedicated EAL interrupt thread, it won't be

> > expected to exit for all kinds of the cause.

> > On this view, I'm in favor of your patch which cancel the interrupt

> > thread, but don't directly return by the EINTR.

> >

> > >

> > >> Without setting 'PTHREAD_CREATE_DETACHED' won't cause the infinite

> > >> loop. However by using pthread_cancel to terminate the thread,

> > >> indeed it's necessary to set 'PTHREAD_CREATE_DETACHED'.

> > > My general understanding is that PTHREAD_CREATE_DETACHED should be

> used for

> > > any thread, which should not keep a process open by itself if it is executing,

> > > i.e. a "daemon thread". I believe the interrupt thread qualifies as such a

> > > thread if I have understood everything right (which is hard to promise when

> > > you only work in DPDK in spare time).

> > >

> > >> It looks like 'pthread_cancel' is the right way and I saw it

> > >> continue keeps current EINTR handling in EAL interrupt thread.

> > > It is one option. Depending what makes the most sense.

> > >

> > >> 1. Can you explain and add patch comments why default signal handler

> > >> is not good enough to terminate app.

> > > Yes if someone call tell me more about what it is so I can check it.

> > >

> > >> 2. I propose to add addition comments on rte_epoll_wait() API

> > >> description. For any signal, it causes an error return, user needs

> > >> to handle.

> > > Agreed.

> > >

> > >> 3. Will you do a favorite to add 'PTHREAD_CREATE_DETACHED' to all

> > >> EAL pthread too.

> > > As a spare time developer I am a bit conservative about too large of a scope

> > > and messing with code for other threads or features I didn't personally use or

> > > test. This is because I don't have the same QA resources as Intel / 6WIND /

> > > etc.. Some help from a full time developer would be great here.

> > All right, reasonable to me.

> >

> > >

> > >> Cunming

> > > Matthew.

> >

>


^ permalink raw reply	[flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH 1/3] rte_interrupts: add rte_eal_intr_exit to shut down IRQ thread
@ 2016-02-13 21:38 Matthew Hall
  2016-03-17 22:55 ` [dpdk-dev] [dpdk-dev, " Matthew Hall
  0 siblings, 1 reply; 7+ messages in thread
From: Matthew Hall @ 2016-02-13 21:38 UTC (permalink / raw)
  To: dev

There is no good way to shut down this thread from an application signal
handler. Here we add an rte_eal_intr_exit() function to allow this.

Signed-off-by: Matthew Hall <mhall@mhcomputing.net>
---
 lib/librte_eal/common/include/rte_eal.h      |  9 +++++++++
 lib/librte_eal/linuxapp/eal/eal_interrupts.c | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index d2816a8..1533eeb 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -165,6 +165,15 @@ int rte_eal_init(int argc, char **argv);
 typedef void	(*rte_usage_hook_t)(const char * prgname);
 
 /**
+ * Shut down the EAL interrupt thread.
+ *
+ * This function can be called from a signal handler during application
+ * shutdown.
+ *
+ */
+int rte_eal_intr_exit(void);
+
+/**
  * Add application usage routine callout from the eal_usage() routine.
  *
  * This function allows the application to include its usage message
diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index b33ccdb..aa332a1 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -892,6 +892,17 @@ rte_eal_intr_init(void)
 		if (ret_1 != 0)
 			RTE_LOG(ERR, EAL,
 			"Failed to set thread name for interrupt handling\n");
+
+int
+rte_eal_intr_exit(void)
+{
+	int ret = 0;
+
+	ret = pthread_cancel(intr_thread);
+	if (ret != 0) {
+		RTE_LOG(ERR, EAL,
+			"Failed to cancel thread for interrupt handling\n");
+		return -ret;
 	}
 
 	return -ret;
-- 
2.5.0

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

end of thread, other threads:[~2020-08-13 22:29 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-13 22:28 [dpdk-dev] [dpdk-dev, 1/3] rte_interrupts: add rte_eal_intr_exit to shut down IRQ thread Bly, Mike
  -- strict thread matches above, loose matches on Subject: below --
2016-02-13 21:38 [dpdk-dev] [PATCH " Matthew Hall
2016-03-17 22:55 ` [dpdk-dev] [dpdk-dev, " Matthew Hall
2016-03-21  7:58   ` Liang, Cunming
2016-03-22  7:39     ` Matthew Hall
2016-03-23  3:24       ` Liang, Cunming
2016-07-08 17:36         ` Thomas Monjalon
2016-07-11  4:07           ` Liang, Cunming

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