On Wed, Dec 4, 2013 at 4:36 PM, Pravin Shelar <pshe...@nicira.com> wrote:
> On Wed, Dec 4, 2013 at 4:23 PM, Jesse Gross <je...@nicira.com> wrote:
>> On Wed, Dec 4, 2013 at 3:42 PM, Pravin Shelar <pshe...@nicira.com> wrote:
>>> On Wed, Dec 4, 2013 at 1:08 PM, Jesse Gross <je...@nicira.com> wrote:
>>>> On Fri, Nov 22, 2013 at 1:20 PM, Pravin B Shelar <pshe...@nicira.com>
>>>> wrote:
>>>>> diff --git a/datapath/flow.h b/datapath/flow.h
>>>>> index 6b68cf1..2405638 100644
>>>>> --- a/datapath/flow.h
>>>>> +++ b/datapath/flow.h
>>>>> -struct sw_flow_stats {
>>>>> +struct flow_stats {
>>>>> u64 packet_count; /* Number of packets matched. */
>>>>> u64 byte_count; /* Number of bytes matched. */
>>>>> unsigned long used; /* Last used time (in jiffies). */
>>>>> spinlock_t lock; /* Lock for atomic stats update.
>>>>> */
>>>>> +};
>>>>> +
>>>>> +struct sw_flow_stats {
>>>>> __be16 tcp_flags; /* Union of seen TCP flags. */
>>>>> -} ____cacheline_aligned_in_smp;
>>>>> + bool is_percpu;
>>>>> + union {
>>>>> + struct flow_stats *stat;
>>>>> + struct flow_stats __percpu *cpu_stats;
>>>>> + };
>>>>> +};
>>>>
>>>> I wonder if it makes sense to directly put struct flow_stats in the
>>>> union for the non-percpu case and avoid an extra indirection there.
>>>>
>>> This saves some memory, thats why I done it this way.
>>
>> It does in the percpu case, although we're essentially already betting
>> that there are relatively few flows there. I think it saves some
>> memory in the non-percpu case though.
>
> I also wanted to make better use of cache-line (as it is accessed in
> hot-path) for per cpu case.
Isn't this on the end of a struct that is just malloc'ed and therefore
not necessarily contiguous with anything of interest?
I'm not trying to convince you to change, just trying to better
understand the rationale.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
