DPDK patches and discussions
 help / color / mirror / Atom feed
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
Date: Mon, 11 Jul 2016 04:07:29 +0000	[thread overview]
Message-ID: <D0158A423229094DA7ABF71CF2FA0DA31554689D@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <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.
> >
> 

  reply	other threads:[~2016-07-11  4:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-13 21:38 [dpdk-dev] [PATCH " Matthew Hall
2016-02-13 21:38 ` [dpdk-dev] [PATCH 2/3] eal_interrupts: mark EAL interrupt thread as a daemon thread Matthew Hall
2016-02-13 21:38 ` [dpdk-dev] [PATCH 3/3] rte_epoll_wait: allow EINTR to be passed to caller Matthew Hall
2016-02-28 21:17 ` [dpdk-dev] [PATCH 1/3] rte_interrupts: add rte_eal_intr_exit to shut down IRQ thread Thomas Monjalon
2016-03-08 15:09   ` Thomas Monjalon
2016-03-09  9:05 ` Liang, Cunming
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 message]
2020-08-13 22:28 Bly, Mike

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=D0158A423229094DA7ABF71CF2FA0DA31554689D@shsmsx102.ccr.corp.intel.com \
    --to=cunming.liang@intel.com \
    --cc=dev@dpdk.org \
    --cc=mhall@mhcomputing.net \
    --cc=thomas.monjalon@6wind.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).