<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/4465/">https://reviewboard.asterisk.org/r/4465/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 10th, 2015, 1:26 p.m. UTC, <b>Ed Hynan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/4465/diff/1/?file=71888#file71888line240" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/res/res_timing_kqueue.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static struct ast_timing_interface kqueue_timing = {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">219</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="tb"> </span><span class="k">case</span> <span class="n">EVFILT_USER</span>:</pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Portability: at least EVFILT_USER and NOTE_TRIGGER are not defined on all kqueue(2) systems.</pre>
</blockquote>
<p>On March 11th, 2015, 4:31 a.m. UTC, <b>Justin T. Gibbs</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Which platforms are you referring to? OS-X added support in 10.6. Why they haven't updated their man pages is anyones guess:
http://www.opensource.apple.com/source/xnu/xnu-1699.24.23/tools/tests/xnu_quick_test/kqueue_tests.c</pre>
</blockquote>
<p>On March 11th, 2015, 9:51 a.m. UTC, <b>Ed Hynan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">OpenBSD and NetBSD. These do not have the user event filter.</pre>
</blockquote>
<p>On March 12th, 2015, 5:29 p.m. UTC, <b>Ed Hynan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I adapted a test program so see if uncollected EVFILT_TIMER events can make poll() return -- reliably.
After testing on FreeBSD 9.0 and OpenBSD 5.5 (leaving NetBSD alone unless something seemed promising),
I find that both will fail to return from poll initially; also both can be 'kicked' with a signal or
a EVFILT_READ event -- but the result differs on the two systems, and this is undocumented and almost
certainly undefined behavior and a side effect (and I assume the EVFILT_USER event was just a similar
'kick').
I haven't studied this timing code, so I shouldn't say much, but *if* res_timing_kqueue.c expects to
return a kqueue fd for use with poll, and that poll will wake for EVFILT_TIMER expire events, then
that won't work (judging by the test program trying to do just that). The technique in res_timing_pthread.c
looks promising: return the read end of a pipe, signal poll by writing a byte, unsignal by consuming
the byte (works in the test), and just using EVFILT_TIMER for the ticks.
</pre>
</blockquote>
<p>On March 13th, 2015, 2:53 a.m. UTC, <b>Justin T. Gibbs</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'm not sure what you mean by 'kick'. I believe that EVFILT_TIMER events are reliable. If the application is slow to consume timer events, the kevent will accumulate them and the file descriptor for the kqueue() cause poll to return immediately.
The goal of this change is to ensure that continuous mode is "cheap" and activates the file descriptor immediately. Setting the timer to run as quickly as possible just burns CPU cycles and means continuous mode blocks until the timer goes off at least once. Depending on the timer resolution of the system, this could be a long time. I need to do some more testing with the original driver to quantify this, but I believe this was the cause of my failures on FreeBSD.
I coded up a version to use EVFILT_READ on a file descriptor of a closed pipe. This worked on FreeBSD. But since it consumes another fd, I think its still better to use EVFLT_USER if it is available. </pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes, EVFILT_TIMER events are reliable. I mean the interaction of kqueue with a timer and poll -- the expectation
that poll will return for a kqueue timer expiration. kqueue_timer_fd(), assigned to
ast_timing_interface kqueue_timing.timer_fd in svn trunk source, returns the kqueue descriptor (timer->handle) --
this code was not present in 11.* sources, so I haven't had the possible problems, but *if* the descriptor
returned from ast_timing_interface kqueue_timing.timer_fd() is used with poll (e.g. main/timing.c), expectations
might/will fail to be met.
By "kick" I mean that in tests after poll initially fails to return for kqueue timer expiration, some other events
e.g. a signal might be followed by poll suddenly starting to return for kqueue timer expiration -- like
something stuck was kicked loose. My guess is that the user event you added was doing just that; but it seems to
be unexpected side effect behavior.
Regardless of all that, the idea that it's "better to use EVFLT_USER if it is available" would lead to
substantially different code for the kqueue systems. Of course, it's up to asterisk-devs whether that
is acceptable (I'm a contributor).</pre>
<br />
<p>- Ed</p>
<br />
<p>On March 13th, 2015, 2:07 a.m. UTC, Justin T. Gibbs wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Justin T. Gibbs.</div>
<p style="color: grey;"><i>Updated March 13, 2015, 2:07 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-24857">ASTERISK-24857</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Update the kqueue timing module to conform to current timer API.
This fixes issues with using the kqueue timing source on Asterisk 13
on FreeBSD 10.
res_timing_kqueue.c:
Remove support for kevent64(). The values used to support Asterisk
timers fit within 32bits and so can be handled on all platforms via
kevent().
Provide debug logging for, but do not track, unacked events. This
matches the behavior of all other timer implementations.
Implement continuous mode by triggering and leaving active, a user
event. This ensures that the file descriptor for the timer returns
immediately from poll(), without placing the load of a high speed
timer on the kernel.
In kqueue_timer_get_max_rate(), don't overstate the capability of
the timer. On some platforms, UINT_MAX is greater than INTPTR_MAX,
the largest integer type kqueue supports for timers.
In kqueue_timer_get_event(), assume the caller woke up from poll()
and just return the mode the timer is currently in. This matches
all other timer implementations.
Adjust the test code now that unacked events are not tracked.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Asterisk 13.2.0 on FreeBSD 10-stable: "timing test", pjsip incoming/outgoing calls, voicemail prompts and recordings. All of the above failed without these changes.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/res/res_timing_kqueue.c <span style="color: grey">(432637)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/4465/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>