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 A62202BD0 for ; Mon, 21 Mar 2016 08:58:45 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP; 21 Mar 2016 00:58:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,370,1455004800"; d="scan'208";a="938189520" Received: from shwdeisgchi017.ccr.corp.intel.com (HELO [10.239.66.70]) ([10.239.66.70]) by orsmga002.jf.intel.com with ESMTP; 21 Mar 2016 00:58:43 -0700 From: "Liang, Cunming" To: Matthew Hall References: <1455399524-3252-1-git-send-email-mhall@mhcomputing.net> <20160317225531.GA10279@mhcomputing.net> Cc: thomas.monjalon@6wind.com, dev@dpdk.org Message-ID: <56EFA9B4.9080404@intel.com> Date: Mon, 21 Mar 2016 15:58:44 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <20160317225531.GA10279@mhcomputing.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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, 21 Mar 2016 07:58:46 -0000 Hi Matthew, On 3/18/2016 6:55 AM, Matthew Hall wrote: > From Cunming: >> I'm trying to understand the motivation. >> >> I don't think you're going to gracefully exit intr thread but leave all >> other eal threads live. We don't have API to new launch intr thread again. > The doc comment added for rte_eal_intr_exit already explains this. According > to the doc I wrote, use of the function is limited to shutting everything > down. > >> So I guess your app is using own pthread(none EAL thread), you're trying to >> safely shutdown the whole application by your signal handler. > No, the app is using DPDK pthreads, and trying to shutdown everything safely > and cleanly w/ its signal handler, across DPDK and many other services in the > app. Get you. You don't satisfy with the default termination signal handler(SIG_DEL). The purpose is to safely clean everything by self-defined signal handler. Can you share us more of your observation on why the default termination handler is not enough/safe? As some of the samples are using it to terminate app, your concern may be necessary to apply on them as well. > Unfortunately, right now from my experience it is impossible to get everything to > cleanly shutdown, one an interrupt thread is activated. Because interrupt > threads violate violate POSIX semantics: > > 1) It ignores EINTR and immediately forcibly restarts a poll() syscall. If the > signal is delivered to the interrupt thread of the process by the kernel, this > makes the thread uninterruptible to process the signal. Stuck running forever. If EINTR is caused by some non-term purpose signals, are you going to exit the interrupt thread any way? > 2) It does not properly set PTHREAD_CREATE_DETACHED for a background thread. > So it holds the process open for its infinite loop of poll(). Stuck running > forever. 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'. > 3) There is no way to access the thread_id from intr_thread. So then you can't > call pthread_cancel on it to shut it down. Stuck running forever. It looks like 'pthread_cancel' is the right way and I saw it continue keeps current EINTR handling in EAL interrupt thread. >> For this purpose, the device shall close safely(turn off intr) during the >> time, intr thread still wait but no event will be raised. > In theory yes. In practice no. Because the intr thread violated POSIX rules > for background processing threads per above. > >> In this view, it seems not necessary to have this new. Can you explain more >> detail for the purpose? > Based on my testing, I disagree. I could not get reliable shutdowns without > this, or I wouldn't have coded it. (: Now it's clear to me, overall it's fine. Three additional comments. 1. Can you explain and add patch comments why default signal handler is not good enough to terminate app. 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. 3. Will you do a favorite to add 'PTHREAD_CREATE_DETACHED' to all EAL pthread too. Cunming > Matthew. > >