On Fri, Oct 10, 2014 at 09:56:45PM +0000, Daniele Di Proietto wrote:
>
> On 10/10/14, 9:35 AM, "Ben Pfaff" <[email protected]> wrote:
> >I think that you could get atomic clearing by replacing the "clear" bool
> >by a uint32_t that is a sequence number. Calling "clear" would just
> >increment the thread_stats's sequence number. Calls to
> >ovsthread_stats_get_bucket() would compare the bucket's sequence number
> >against the thread_stats's sequence number and clear the data if they
> >differ. Calls to ovsthread_stats_aggregate() could detect that the
> >stats were cleared during iteration by noticing that the thread_stats's
> >sequence number changed from the beginning to the end.
>
> I'm not sure I understand your suggestion. The problem that I see with
> clear() operation is that it sets the 'clear' flag on the threads' bucket
> (struct ovsthread_stats_bucket is thread local) one at a time. Therefore
> an "aggregator" might see some of the buckets with the 'clear' flags set
> and the others without the 'clear' flags set, because
> ovsthread_stats_clear() is in the process of setting the 'clear' flag on
> all the buckets.
>
> Are you suggesting to add a global (non per-thread), per-id counter?
Here is some more detail of my idea.
We would need one counter per thread stats id. It would probably be
easiest to declare a new struct and push the existing "int id;" and
the new counter in it. (This would also add a measure of type safety
for the thread stats and so it's not a bad idea even if there were
only the "int id;" in it.)
ovsthread_stats_clear() would just increment the new counter added to
the new struct.
In struct ovsthread_stats_bucket, replace "bool clear;" by
"atomic_uint32_t clear_seq;".
In ovsthread_stats_get_bucket(), change
if (ret->clear) {
memset(&ret->data, 0, sizeof ret->data);
ret->clear = false;
}
to something like:
atomic_read_explicit(&ret->clear_seq, &seq1);
atomic_read_explicit(&new_struct->new_counter, &seq2);
if (seq1 != seq2) {
memset(&ret->data, 0, sizeof ret->data);
atomic_store_explicit(&ret->clear_seq, &seq2, memory_order_release);
}
ovsthread_stats_aggregate() would read the newly added counter before
and after aggregating. If it changes, then it would need to start
over (or perhaps just report all-zeros).
I don't know whether this problem (of non-atomic clearing) is
important enough to solve.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
