On Mon, Feb 10, 2014 at 11:15 AM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Thu, Feb 6, 2014 at 3:13 PM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
>> @@ -80,96 +76,126 @@ void ovs_flow_stats_update(struct sw_flow *flow, struct
>> sk_buff *skb)
>> tcp_flags = TCP_FLAGS_BE16(tcp_hdr(skb));
>> }
>>
>> - spin_lock(&stats->lock);
>> + /* Check if already have node-specific stats. */
>> + if (likely(stats))
>> + spin_lock(&stats->lock);
>> + else {
>> + stats = flow->stats[0]; /* Pre-allocated. */
>> + spin_lock(&stats->lock);
>> +
>> + /* If the current NUMA-node is the only writer on the
>> + * pre-allocated stats keep using them.
>> + * A previous locker may have already allocated the stats,
>> + * so we need to check again. If the node-specific stats
>> + * were already allocated, we update the pre-allocated stats
>> + * as we have already locked them. */
>> + if (likely(stats->last_writer != node && stats->last_writer
>> >= 0
>> + && !flow->stats[node])) {
>> + /* Try to allocate node-specific stats. */
>> + struct flow_stats *new_stats;
>> +
>> + new_stats = kmem_cache_alloc_node(flow_stats_cache,
>> + GFP_THISNODE |
>> + __GFP_NOMEMALLOC,
>> + node);
>> + if (likely(new_stats)) {
>> + new_stats->used = jiffies;
>> + new_stats->packet_count = 1;
>> + new_stats->byte_count = skb->len;
>> + new_stats->tcp_flags = tcp_flags;
>> + new_stats->last_writer = node;
>> + spin_lock_init(&new_stats->lock);
>> +
>> + flow->stats[node] = new_stats;
>> + goto unlock; /* Unlock the pre-allocated
>> stats. */
>
> One more thing I noticed is that we need memory barrier for numa-stats
> array. Since other process could be reading starts from different cpu.
It would be good if we could use some higher level construct that
contains memory barriers (like a lock). Raw memory barriers are
notoriously difficult to reason about.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
