[ovs-dev] [PATCH v5 2/2] datapath: Per NUMA node flow stats.

Jarno Rajahalme jrajahalme at nicira.com
Tue Feb 11 00:43:31 UTC 2014


On Feb 10, 2014, at 3:28 PM, Jesse Gross <jesse at nicira.com> wrote:

> On Mon, Feb 10, 2014 at 11:15 AM, Pravin Shelar <pshelar at nicira.com> wrote:
>> On Thu, Feb 6, 2014 at 3:13 PM, Jarno Rajahalme <jrajahalme at 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.


I think I nailed it, so let me know what you think. I just posted v6.

  Jarno





More information about the dev mailing list