Hi Ben, 

>On Thu, Jun 22, 2017 at 10:10:49PM +0100, Bhanuprakash Bodireddy wrote:
>> pkt_metadata_init() is called for every packet in userspace datapath
>> and initializes few members in pkt_metadata. Before this the members
>> that needs to be initialized are prefetched using
>pkt_metadata_prefetch_init().
>>
>> The above functions are critical to the userspace datapath performance
>> and should be in sync. Any changes to the pkt_metadata should also
>> include changes to metadata_init() and prefetch_init() if necessary.
>>
>> This commit slightly refactors the pkt_metadata structure and
>> introduces cache line markers to catch any violations to the
>> structure. Also only prefetch the cachelines having the members that needs
>to be zeroed out.
>>
>> Signed-off-by: Bhanuprakash Bodireddy
>> <bhanuprakash.bodire...@intel.com>
>
>OVS has a PADDED_MEMBERS macro that makes this easier.  Unfortunately it
>isn't currently adequate for use more than once per struct.  But it's fixable, 
>so I
>sent a patch to fix it:
>        https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335204.html

Thanks for this patch. PADDED_MEMBERS macro is pretty handy. 

>and a fixed-up version of the original patch:
>        https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335205.html

Thanks  for improving the patch. 

>
>However, even with the fix, this is going to cause problems with MSVC,
>because it does not allow 0-length arrays.  Maybe you can find another way to
>mark the beginning of a cache line.

Microsoft links mentions that the warning "zero-sized array in struct/union" we 
encountered
is a 'Level-4' warning and is numbered 'C4200'. 

C4200: https://msdn.microsoft.com/en-us/library/79wf64bc.aspx

I can't think of alternate ways to mark the cachelines, so how about this 
incremental change in lib/util.h
that disables the warning when building with MSVC?

I am currently on vacation and have limited access to lab to download and test 
the changes with Microsoft compiliers.
Apologies for this.

--8<--------------------------cut here-------------------------->8--

#define CACHE_LINE_SIZE 64
BUILD_ASSERT_DECL(IS_POW2(CACHE_LINE_SIZE));
#ifndef _MSC_VER
typedef void *OVS_CACHE_LINE_MARKER[0];
#else
__pragma (warning(push))
__pragma (warning(disable:4200))
typedef void *OVS_CACHE_LINE_MARKER[0];
__pragma (warning(pop))
#endif
 
- Bhanuprakash.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to