From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 0B1D41396 for ; Mon, 11 Jul 2016 06:07:32 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga103.jf.intel.com with ESMTP; 10 Jul 2016 21:07:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,344,1464678000"; d="scan'208";a="992588659" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga001.jf.intel.com with ESMTP; 10 Jul 2016 21:07:31 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 10 Jul 2016 21:07:31 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.248.2; Sun, 10 Jul 2016 21:07:30 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.147]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.8]) with mapi id 14.03.0248.002; Mon, 11 Jul 2016 12:07:28 +0800 From: "Liang, Cunming" To: Thomas Monjalon CC: Matthew Hall , "dev@dpdk.org" Thread-Topic: [dpdk-dev,1/3] rte_interrupts: add rte_eal_intr_exit to shut down IRQ thread Thread-Index: AQHRgKAnWNtXOL4B5ESY/APzikY1eJ9jjboAgAEGzoCAAdEgAICokWWAgARX/rA= Date: Mon, 11 Jul 2016 04:07:29 +0000 Message-ID: References: <1455399524-3252-1-git-send-email-mhall@mhcomputing.net> <20160322073921.GA28285@mhcomputing.net> <56F20C56.9050801@intel.com> <7816883.NtLY7W8fN8@xps13> In-Reply-To: <7816883.NtLY7W8fN8@xps13> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [dpdk-dev, 1/3] rte_interrupts: add rte_eal_intr_exit to shut down IRQ thread X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 Jul 2016 04:07:33 -0000 Hi Thomas, Base on the previous conversation, at least it requires v2 to reword some c= omments. > > >> 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 > Cc: Matthew Hall ; dev@dpdk.org > Subject: Re: [dpdk-dev,1/3] rte_interrupts: add rte_eal_intr_exit to shut= down > IRQ thread >=20 > Cunming, what is the status of this patchset, please? >=20 > 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 loop= ing. > > 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 e= xecuting, > > > i.e. a "daemon thread". I believe the interrupt thread qualifies as s= uch a > > > thread if I have understood everything right (which is hard to promis= e 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 personal= ly use or > > > test. This is because I don't have the same QA resources as Intel / 6= WIND / > > > etc.. Some help from a full time developer would be great here. > > All right, reasonable to me. > > > > > > > >> Cunming > > > Matthew. > > >=20