Review Board 1.7.16


timerfd timing interface

Review Request #52 - Created Nov. 11, 2008 and submitted

Mark Michelson
/trunk
Reviewers
asterisk-dev
kpfleming, russell
Asterisk
Starting with Linux Kernel version 2.6.25 and glibc version 2.8, there is a timing interface known as timerfd. This review request encompasses an Asterisk timing implementation using the timerfd interface.

Of particular note in this review request are all things related to menuselect and autoconf. If there's anything there that seems wrong or out of place, please don't hesitate to yell about it. Also, I'm not sure what all the changes to ast_expr2.[hc] are about, but if I've done something wrong with regards to those, please let me know how I can fix those too.
I have tested this by using the "timing test" CLI command and by doing some file playback.
Review request changed
Updated (Nov. 12, 2008, 11:07 a.m.)
Try again
Ship it!
Posted (Nov. 12, 2008, 11:53 a.m.)
You accidentally duplicated the content of res_timer_timerfd.c in the diff but other than that (and the trailing whitespace which is made so obvious by reviewboard) it looks good to me.
Ship it!
Posted (Nov. 18, 2008, 12:50 p.m.)
All of my comments are minor, so I don't feel that I need to look at this again.  Commit away.  Nice work.

On a related note, I think we should add a section to the README for Asterisk, or somewhere obvious, that describes the situation with the timing modules.  We should list each one, and some basic guidelines for people to know which one they should use.
/trunk/configure.ac (Diff revision 5)
 
 
As we discussed via email, you're missing AST_EXT_LIB_SETUP() in this file, but I think you already committed that fix.
/trunk/res/res_timing_timerfd.c (Diff revision 5)
 
 
I would change this to:

continuous_timer = {
    .it_value.tv_nsec = 1L,
};

Then, you can remove the memset() from later in the function.
/trunk/res/res_timing_timerfd.c (Diff revision 5)
 
 
You have trailing whitespace around the patch
/trunk/res/res_timing_timerfd.c (Diff revision 5)
 
 
 
 
 
 
 
 
 
For some bizarre reason, your module was included in this diff multiple times.  I'm curious how this happened ...

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.