[ovs-dev] [PATCH 1/2] coverage: Reimplement the "ovs-appctl coverage/show" command.

Alex Wang alexw at nicira.com
Thu Sep 26 15:35:28 UTC 2013


Thanks for the review,

On Wed, Sep 25, 2013 at 6:11 PM, Ethan Jackson <ethan at nicira.com> wrote:

> Couple of high level questions about this.
>
> Instead of protecting coverage_run() with a mutex and calling it in
> time_poll(), why not just call it once at the beginning of the main
> thread's run loop?  If we do that, we can avoid all the locking which
> could affect the threads.
>


Ben explained the need to invoke this every thread to me here:
http://openvswitch.org/pipermail/dev/2013-August/031166.html

Since we have other daemons who also use the coverage/counter, it is
easier to just call it from time_poll().  The alternative is to register
the main
thread id in coverage.c and only allow the main thread to do it.

I'd like to make the change if you think it is necessary.



> Ben, could we do the same for coverage_clear()?  It seems a bit odd to
> me that time_poll() calls this stuff in a multithreaded environment.
> That said, perhaps we should leave it as it is, and do something more
> comprehensive to deal with multiple threads later.
>


I think coverage_clear() should be invoked by each thread.  since it is for
adding the per-thread count to the total count.



> When you compute "slots", why is it possible that the number of run
> intervals is a multiple of COVERAGE_RUN_INTERVAL?  I.E. why isn't
> slots always equal to 1?
>

Since it did not register a wake up time, so, at worst case, it could be
greater than 1.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130926/96afb702/attachment-0003.html>


More information about the dev mailing list