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

[Ben Pfaff]

On Tue, Aug 27, 2013 at 01:13:11PM -0700, Alex Wang wrote:
> Thanks a lot for the info.
> 
> For you review, I still have one issue (buried in my emails):
> 
> """
> 
> > +/* Index counter used to compute the moving average array's index. */
> > > +static unsigned int idx_count = 0;
> >
> > It looks to me like idx_count should be protected by coverage_mutex.
> 
> 
> 
> I'm a bit confused here. Want to ask, that there will only be one thread
> going through the loop in ovs-vswitchd.c (which calls coverage_run()),
> right? If so, idx_count will only be accessed by that thread. Then do we
> still need to protect it?

I missed that only one thread calls coverage_run().  Now that I
understand the intent, I see some other issues to resolve.  First,
this intent isn't documented, but it should be (at least as a comment
on coverage_run().)

Second, the patch only modifies ovs-vswitchd to call coverage_run().
We have other daemons that also should presumably call it.  One can
modify each of them to call coverage_run(), but it would be better, if
possible, to avoid the need for them to manually call it, because it
is harder to miss in that case.  One way to do that would be to call
it automatically from poll_block().  Either it could be made possible
to call coverage_run() from multiple threads, or we could devise a way
to have poll_block() or coverage_run() figure out which thread is the
"main thread".

Thanks,

Ben.