[ovs-dev] [PATCH 1/4] Make sure that time advances in a daemon between calls to time_refresh().

[Ben Pfaff]

OK, I made a static function with a more straightforward name and
had time_init() and time_postfork() both call that.

Pushed to citrix.

Justin Pettit <jpettit at nicira.com> writes:

> Great catch!  My only nit is that the function is called time_postfork
> (), but it is called in time_init(), which is definitely pre-fork.
> Kind of like high school.  :-(
>
> --Justin
>
>
> On Oct 14, 2009, at 4:54 PM, Ben Pfaff wrote:
>
>> Open vSwitch uses an interval timer signal to tell it that its
>> cached idea
>> of the current time has expired.  However, this didn't work in a
>> daemon
>> detached from the foreground session (invoked with --detach) because a
>> child created with fork() does not inherit the parent's interval
>> timer and
>> we did not re-set it after calling fork().
>>
>> This commit fixes the problem by setting the interval timer back up
>> after
>> calling fork() from daemonize().
>>
>> This fix is based on code inspection (which was then verified to be
>> correct
>> through testing).  It may not fix any actual problems in practice,
>> because
>> time_refresh() is called every time through the poll loop, and the
>> poll
>> loop typically runs more quickly than the periodic timer fires (1 ms
>> or so
>> average in ovs-vswitchd, vs. 100 ms timer interval).
>> ---
>> lib/daemon.c  |    2 ++
>> lib/timeval.c |   17 ++++++++++++++---
>> lib/timeval.h |    1 +
>> 3 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/daemon.c b/lib/daemon.c
>> index 1e3f002..a35c639 100644
>> --- a/lib/daemon.c
>> +++ b/lib/daemon.c
>> @@ -23,6 +23,7 @@
>> #include <unistd.h>
>> #include "fatal-signal.h"
>> #include "dirs.h"
>> +#include "timeval.h"
>> #include "util.h"
>>
>> #define THIS_MODULE VLM_daemon
>> @@ -222,6 +223,7 @@ daemonize(void)
>>             if (chdir_) {
>>                 chdir("/");
>>             }
>> +            time_postfork();
>>             break;
>>
>>         case -1:
>> diff --git a/lib/timeval.c b/lib/timeval.c
>> index 3cca338..c62f519 100644
>> --- a/lib/timeval.c
>> +++ b/lib/timeval.c
>> @@ -57,8 +57,6 @@ void
>> time_init(void)
>> {
>>     struct sigaction sa;
>> -    struct itimerval itimer;
>> -
>>     if (inited) {
>>         return;
>>     }
>> @@ -78,7 +76,20 @@ time_init(void)
>>         ovs_fatal(errno, "sigaction(SIGALRM) failed");
>>     }
>>
>> -    /* Set up periodic timer. */
>> +    /* Set up periodic signal. */
>> +    time_postfork();
>> +}
>> +
>> +/* Set up the interval timer, to ensure that time advances even
>> without calling
>> + * time_refresh().
>> + *
>> + * A child created with fork() does not inherit the parent's
>> interval timer, so
>> + * this function needs to be called from the child after fork(). */
>> +void
>> +time_postfork(void)
>> +{
>> +    struct itimerval itimer;
>> +
>>     itimer.it_interval.tv_sec = 0;
>>     itimer.it_interval.tv_usec = TIME_UPDATE_INTERVAL * 1000;
>>     itimer.it_value = itimer.it_interval;
>> diff --git a/lib/timeval.h b/lib/timeval.h
>> index 660a207..8567d75 100644
>> --- a/lib/timeval.h
>> +++ b/lib/timeval.h
>> @@ -41,6 +41,7 @@ BUILD_ASSERT_DECL(TYPE_IS_SIGNED(time_t));
>> #define TIME_UPDATE_INTERVAL 100
>>
>> void time_init(void);
>> +void time_postfork(void);
>> void time_refresh(void);
>> time_t time_now(void);
>> long long int time_msec(void);
>> --
>> 1.6.3.3
>>
>>
>> _______________________________________________
>> dev mailing list
>> dev at openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org