On Mon, Feb 10, 2014 at 11:31 AM, Jarno Rajahalme <jrajaha...@nicira.com> wrote:
> On Feb 8, 2014, at 7:01 PM, Jesse Gross <je...@nicira.com> wrote:
>> On Thu, Feb 6, 2014 at 3:13 PM, Jarno Rajahalme <jrajaha...@nicira.com>
>> wrote:
>>> diff --git a/datapath/flow.h b/datapath/flow.h
>>> index eafcfd8..f6cce35 100644
>>> --- a/datapath/flow.h
>>> +++ b/datapath/flow.h
>>> @@ -155,14 +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. */
>>> -};
>>> -
>>> -struct sw_flow_stats {
>>> - bool is_percpu;
>>> - union {
>>> - struct flow_stats *stat;
>>> - struct flow_stats __percpu *cpu_stats;
>>> - };
>>> + int last_writer; /* NUMA-node id of the last writer
>>> or
>>> + * -1. Meaningful for 'stats[0]'
>>> only.
>>> + */
>>
>> Should we put last_writer directly in struct sw_flow stats? It would
>> reduce the size a little bit and might even be a little clearer.
>>
>
> This patch actually removes the struct sw_flow_stats, and the ‘last_writer’
> is part of the struct flow_stats. The alternative to put it to struct sw_flow
> would use more space (flow_stats are allocated cache line aligned, so there
> is some slack), and make the struct sw_flow a bit more volatile than it needs
> to be. To be clear, the definitions now look like this:
Sorry, that was a typo. I meant to say put it in struct sw_flow as you guessed.
It sounds like you are planning on removing the cacheline alignment
for flow_stats, so moving it would help save a little space. As far as
writing to sw_flow frequently, I guess this would primarily happen if
we have an allocation failure right (since if it wasn't in struct
flow_stats then we probably wouldn't access it on every stats update
anyways)? Is it possible to minimize the effects in this corner case?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
