On Thu, Jun 12, 2014 at 3:35 PM, Ben Pfaff <b...@nicira.com> wrote:
> On Fri, Jun 06, 2014 at 10:04:31AM -0700, Gurucharan Shetty wrote:
>> The new command is of the form 'time/warp LARGE_MSECS MSECS'.
>> It advances the current monotonic time by LARGE_MSECS. This is done MSECS
>> at a time in each run of the main thread. This gives other threads
>> time to run after the clock has been advanced by MSECS.
>>
>> The old command would continue to work.
>>
>> Rationale: On Windows, process creation is slower. When we have tests
>> that run 'ovs-appctl time/warp MSECS' hundreds of times in a for loop,
>> the time it takes to complete the test increases. This is specially
>> true for bfd tests. For e.g, the 11 bfd tests would take 3.5 minutes
>> to complete before this change and now takes a little less than 2 minutes.
>>
>> Signed-off-by: Gurucharan Shetty <gshe...@nicira.com>
>
> In struct large_warp, it seems a little awkward to store 'warp' as a
> timespec. I am surprised that it is not just an integer number of
> milliseconds.
>
I dilly-dallied a bit on it and was expecting an opinion. I will add
the following addition
and send a V2.
diff --git a/lib/timeval.c b/lib/timeval.c
index 5eed916..53f4faa 100644
--- a/lib/timeval.c
+++ b/lib/timeval.c
@@ -60,7 +60,7 @@ static ULARGE_INTEGER unix_epoch;
struct large_warp {
struct unixctl_conn *conn; /* Connection waiting for warp response. */
long long int total_warp; /* Total offset to be added to monotonic time. */
- struct timespec warp; /* 'total_warp' offset done in steps of 'warp'. */
+ long long int warp; /* 'total_warp' offset done in steps of 'warp'. */
unsigned int main_thread_id; /* Identification for the main thread. */
};
@@ -452,10 +452,17 @@ xclock_gettime(clock_t id, struct timespec *ts)
}
static void
+msec_to_timespec(long long int ms, struct timespec *ts)
+{
+ ts->tv_sec = ms / 1000;
+ ts->tv_nsec = (ms % 1000) * 1000 * 1000;
+}
+
+static void
timewarp_work(void)
{
- long long int warp_in_msecs;
struct clock *c = &monotonic_clock;
+ struct timespec warp;
ovs_mutex_lock(&c->mutex);
if (!c->large_warp.conn) {
@@ -463,20 +470,18 @@ timewarp_work(void)
return;
}
- warp_in_msecs = timespec_to_msec(&c->large_warp.warp);
-
- if (c->large_warp.total_warp - warp_in_msecs >= 0) {
- timespec_add(&c->warp, &c->warp, &c->large_warp.warp);
- c->large_warp.total_warp -= warp_in_msecs;
+ if (c->large_warp.total_warp - c->large_warp.warp >= 0) {
+ msec_to_timespec(c->large_warp.warp, &warp);
+ timespec_add(&c->warp, &c->warp, &warp);
+ c->large_warp.total_warp -= c->large_warp.warp;
} else if (c->large_warp.total_warp) {
- struct timespec warp;
- warp.tv_sec = c->large_warp.total_warp / 1000;
- warp.tv_nsec = (c->large_warp.total_warp % 1000) * 1000 * 1000;
+ msec_to_timespec(c->large_warp.total_warp, &warp);
timespec_add(&c->warp, &c->warp, &warp);
c->large_warp.total_warp = 0;
} else {
/* c->large_warp.total_warp is 0. */
- timespec_add(&c->warp, &c->warp, &c->large_warp.warp);
+ msec_to_timespec(c->large_warp.warp, &warp);
+ timespec_add(&c->warp, &c->warp, &warp);
}
if (!c->large_warp.total_warp) {
@@ -707,7 +712,6 @@ static void
timeval_warp_cb(struct unixctl_conn *conn,
int argc OVS_UNUSED, const char *argv[], void *aux OVS_UNUSED)
{
- struct timespec ts;
long long int total_warp = argc > 2 ? atoll(argv[1]) : 0;
long long int msecs = argc > 2 ? atoll(argv[2]) : atoll(argv[1]);
if (msecs <= 0 || total_warp < 0) {
@@ -715,9 +719,6 @@ timeval_warp_cb(struct unixctl_conn *conn,
return;
}
- ts.tv_sec = msecs / 1000;
- ts.tv_nsec = (msecs % 1000) * 1000 * 1000;
-
ovs_mutex_lock(&monotonic_clock.mutex);
if (monotonic_clock.large_warp.conn) {
ovs_mutex_unlock(&monotonic_clock.mutex);
@@ -727,7 +728,7 @@ timeval_warp_cb(struct unixctl_conn *conn,
atomic_store(&monotonic_clock.slow_path, true);
monotonic_clock.large_warp.conn = conn;
monotonic_clock.large_warp.total_warp = total_warp;
- monotonic_clock.large_warp.warp = ts;
+ monotonic_clock.large_warp.warp = msecs;
monotonic_clock.large_warp.main_thread_id = ovsthread_id_self();
ovs_mutex_unlock(&monotonic_clock.mutex);
> The code seems OK to me. I did not run the tests to make sure that
> they still pass.
>
> Acked-by: Ben Pfaff <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
