[ovs-dev] [PATCH] seq: Attribute wakeups to seq_wait()'s caller, not to seq_wait() itself.

Ben Pfaff blp at nicira.com
Thu May 22 16:40:42 UTC 2014


Thanks!  Applied to master.

On Wed, May 21, 2014 at 01:27:33PM -0700, Jarno Rajahalme wrote:
> LGTM,
> 
>   Jarno
> 
> Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
> 
> > On Apr 30, 2014, at 2:35 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 
> > The poll_loop code has a feature that, when turned on manually or
> > automatically (due to high CPU use), logs the source file and line number
> > of the code that caused a thread to wake up from poll().  Until now, when
> > a function calls seq_wait(), the source file and line number logged was
> > the code inside seq_wait().  seq_wait() has many callers, so that
> > information is not as useful as it could be.  This commit changes the
> > source file and line number used to be that of seq_wait()'s caller.
> > 
> > I found this useful for debugging.
> > 
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> > lib/seq.c |   20 ++++++++++++--------
> > lib/seq.h |    7 +++++--
> > 2 files changed, 17 insertions(+), 10 deletions(-)
> > 
> > diff --git a/lib/seq.c b/lib/seq.c
> > index 7a34244..452d683 100644
> > --- a/lib/seq.c
> > +++ b/lib/seq.c
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2013 Nicira, Inc.
> > + * Copyright (c) 2013, 2014 Nicira, Inc.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -125,7 +125,7 @@ seq_read(const struct seq *seq)
> > }
> > 
> > static void
> > -seq_wait__(struct seq *seq, uint64_t value)
> > +seq_wait__(struct seq *seq, uint64_t value, const char *where)
> >     OVS_REQUIRES(seq_mutex)
> > {
> >     unsigned int id = ovsthread_id_self();
> > @@ -137,7 +137,7 @@ seq_wait__(struct seq *seq, uint64_t value)
> >             if (waiter->value != value) {
> >                 /* The current value is different from the value we've already
> >                  * waited for, */
> > -                poll_immediate_wake();
> > +                poll_immediate_wake_at(where);
> >             } else {
> >                 /* Already waiting on 'value', nothing more to do. */
> >             }
> > @@ -154,7 +154,7 @@ seq_wait__(struct seq *seq, uint64_t value)
> >     list_push_back(&waiter->thread->waiters, &waiter->list_node);
> > 
> >     if (!waiter->thread->waiting) {
> > -        latch_wait(&waiter->thread->latch);
> > +        latch_wait_at(&waiter->thread->latch, where);
> >         waiter->thread->waiting = true;
> >     }
> > }
> > @@ -165,18 +165,22 @@ seq_wait__(struct seq *seq, uint64_t value)
> >  *
> >  * seq_read() and seq_wait() can be used together to yield a race-free wakeup
> >  * when an object changes, even without an ability to lock the object.  See
> > - * Usage in seq.h for details. */
> > + * Usage in seq.h for details.
> > + *
> > + * ('where' is used in debug logging.  Commonly one would use seq_wait() to
> > + * automatically provide the caller's source file and line number for
> > + * 'where'.) */
> > void
> > -seq_wait(const struct seq *seq_, uint64_t value)
> > +seq_wait_at(const struct seq *seq_, uint64_t value, const char *where)
> >     OVS_EXCLUDED(seq_mutex)
> > {
> >     struct seq *seq = CONST_CAST(struct seq *, seq_);
> > 
> >     ovs_mutex_lock(&seq_mutex);
> >     if (value == seq->value) {
> > -        seq_wait__(seq, value);
> > +        seq_wait__(seq, value, where);
> >     } else {
> > -        poll_immediate_wake();
> > +        poll_immediate_wake_at(where);
> >     }
> >     ovs_mutex_unlock(&seq_mutex);
> > }
> > diff --git a/lib/seq.h b/lib/seq.h
> > index c764809..38c0e52 100644
> > --- a/lib/seq.h
> > +++ b/lib/seq.h
> > @@ -1,5 +1,5 @@
> > /*
> > - * Copyright (c) 2013 Nicira, Inc.
> > + * Copyright (c) 2013, 2014 Nicira, Inc.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -111,6 +111,7 @@
> >  */
> > 
> > #include <stdint.h>
> > +#include "util.h"
> > 
> > /* For implementation of an object with a sequence number attached. */
> > struct seq *seq_create(void);
> > @@ -119,7 +120,9 @@ void seq_change(struct seq *);
> > 
> > /* For observers. */
> > uint64_t seq_read(const struct seq *);
> > -void seq_wait(const struct seq *, uint64_t value);
> > +
> > +void seq_wait_at(const struct seq *, uint64_t value, const char *where);
> > +#define seq_wait(seq, value) seq_wait_at(seq, value, SOURCE_LOCATOR)
> > 
> > /* For poll_block() internal use. */
> > void seq_woke(void);
> > -- 
> > 1.7.10.4
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list