On Feb 12, 2014, at 4:12 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Tue, Feb 11, 2014 at 4:07 PM, Jarno Rajahalme <jrajaha...@nicira.com>
> wrote:
...
>> +#endif
>
> How much overhead do we have for non-NUMA systems without this compile
> time check.
> Since we always allocate node zero stat I do not think update stats
> has that much overhead for that case.
>
You are right, I removed this.
...
>>
>> + spin_lock_bh(&stats->lock);
>> + *used = stats->used;
>> + *tcp_flags = stats->tcp_flags;
>> + ovs_stats->n_packets = stats->packet_count;
>> + ovs_stats->n_bytes = stats->byte_count;
>> + spin_unlock_bh(&stats->lock);
>> +
> Is there reason for reading node zero stat outside of for_each_node() {} loop?
>
Not really.
>> +#if MAX_NUMNODES > 1
>> + /* Collect stats from other nodes. */
>> + for_each_node(node) {
>> + if (node == 0)
>> + continue; /* Done already. */
>> + stats = flow->stats[node];
>
> You need smp_read_barrier_depends() barrier here.
> As Jesse suggested we can use higher level of API, like
> rcu_dereference/assign.
>
Changed to use rcu macros.
...
>>
>> static int check_header(struct sk_buff *skb, int len)
>> diff --git a/datapath/flow.h b/datapath/flow.h
>> index c4de0e6..f6cce35 100644
>> --- a/datapath/flow.h
>> +++ b/datapath/flow.h
>> @@ -155,6 +155,9 @@ struct flow_stats {
>> unsigned long used; /* Last used time (in jiffies). */
>> spinlock_t lock; /* Lock for atomic stats update. */
>> __be16 tcp_flags; /* Union of seen TCP flags. */
>> + int last_writer; /* NUMA-node id of the last writer or
>> + * -1. Meaningful for 'stats[0]'
>> only.
>> + */
>> };
>>
> I did not understood your comment from last thread about moving it to
> struct sw_flow.
> Since flow->stat[0] is only shared why we need last_writer in each flow_stats?
> Therefore we should be able to move it to struct sw_flow.
Right. However, the code in ovs_flow_stats_update() becomes a bit more
complicated. Let me know what you think.
To keep things simple I’ll post a new version of this patch separately from the
rest of the series.
Jarno
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
