On Wed, Jul 12, 2017 at 10:38:30AM +0000, Bodireddy, Bhanuprakash wrote:
> 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
> >> <[email protected]>
> >
> >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.

I doubt that this is about a warning, because as I understand it OVS on
MSVC causes a lot of warnings, so it's probably a more serious issue.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to