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

["Liang, Cunming" <cunming.liang@intel.com>]

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