Forgot there are tests in private branch when submit the patch.
The change looks good to me. The only places that acquire rwlock is in
"timeval_stop_cb" and "timeval_warp_cb". So seem to me that, if not in
test, we will always acquire "rdlock()", right? And will that still cause
a lot lock contention (I think it will not)? or add a lot of overhead?
There is a redundant call needs to removed, if we will apply this patch:
(at line 573)
"""
xclock_gettime(monotonic_clock.id, &monotonic_clock.cache);
"""
On Thu, Sep 12, 2013 at 5:46 PM, Ethan Jackson <et...@nicira.com> wrote:
> Assuming we really need this ability. I don't like that we have to
> take a readlock every time we get the time. Perhaps we could could
> have a global flag which is false unless time has ever been warped.
> If it hasn't then we can simply do the xclock_gettime(). I think
> that'd add a slight race, but I can't imagine it'd matter.
>
> Ethan
>
> On Thu, Sep 12, 2013 at 5:24 PM, Ben Pfaff <b...@nicira.com> wrote:
> > Commit 31ef9f5178dee18 (timeval: Remove CACHE_TIME scheme.) inadvertently
> > removed the ability to warp time forward, for use in tests, except when
> > time is stopped. This fixes the problem.
> >
> > Signed-off-by: Ben Pfaff <b...@nicira.com>
> > ---
> > lib/timeval.c | 19 +++++++++++++------
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/timeval.c b/lib/timeval.c
> > index 3262397..7584d58 100644
> > --- a/lib/timeval.c
> > +++ b/lib/timeval.c
> > @@ -105,15 +105,22 @@ time_init(void)
> > static void
> > time_timespec__(struct clock *c, struct timespec *ts)
> > {
> > + struct timespec warp;
> > + struct timespec cache;
> > + bool stopped;
> > +
> > time_init();
> >
> > - if (!c->stopped) {
> > - xclock_gettime(c->id, ts);
> > - } else {
> > - ovs_rwlock_rdlock(&c->rwlock);
> > - timespec_add(ts, &c->cache, &c->warp);
> > - ovs_rwlock_unlock(&c->rwlock);
> > + ovs_rwlock_rdlock(&c->rwlock);
> > + stopped = c->stopped;
> > + warp = c->warp;
> > + cache = c->cache;
> > + ovs_rwlock_unlock(&c->rwlock);
> > +
> > + if (!stopped) {
> > + xclock_gettime(c->id, &cache);
> > }
> > + timespec_add(ts, &cache, &warp);
> > }
> >
> > /* Stores a monotonic timer, accurate within TIME_UPDATE_INTERVAL ms,
> into
> > --
> > 1.7.10.4
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
