[ovs-dev] [PATCH v1] netdev: use acquire-release semantics for change_seq in netdev

Ben Pfaff blp at ovn.org
Tue Nov 26 22:24:20 UTC 2019


Thanks.

On Tue, Nov 26, 2019 at 07:33:30AM +0000, Yanqin Wei (Arm Technology China) wrote:
> Hi Ben,
> 
> Thanks for your time to review. I am sorry not to verify this patch by clang compiler.  But this patch compiles successfully in gcc 7.4. Maybe in some gcc version, the atomic type is necessary for atomic operation.
> I will fix it in V2.
> 
> Best Regards,
> Wei Yanqin
> 
> > -----Original Message-----
> > From: Ben Pfaff <blp at ovn.org>
> > Sent: Tuesday, November 26, 2019 5:54 AM
> > To: Yanqin Wei (Arm Technology China) <Yanqin.Wei at arm.com>
> > Cc: dev at openvswitch.org; ovs-dev at openvswitch.org; nd <nd at arm.com>;
> > Gavin Hu (Arm Technology China) <Gavin.Hu at arm.com>
> > Subject: Re: [ovs-dev] [PATCH v1] netdev: use acquire-release semantics for
> > change_seq in netdev
> > 
> > On Mon, Nov 18, 2019 at 10:46:59AM +0800, Yanqin Wei wrote:
> > > "rxq_enabled" of netdev is writen in the vhost thread and read by pmd
> > > thread once it observes 'change_seq' is updated. This patch is to keep
> > > order on aarch64 or other weak memory model CPU to ensure
> > > 'rxq_enabled' is observed before 'change_seq'.
> > >
> > > Reviewed-by: Gavin Hu <Gavin.Hu at arm.com>
> > > Signed-off-by: Yanqin Wei <Yanqin.Wei at arm.com>
> > 
> > Thanks for the patch.
> > 
> > This does not compile.  Clang says:
> > 
> >     In file included from ../lib/dpif-netdev.c:54:
> >     ../lib/netdev-provider.h:97:5: error: address argument to atomic operation
> > must be a pointer to _Atomic type ('uint64_t *' (aka 'unsigned long *') invalid)
> >     ../lib/ovs-atomic-clang.h:82:16: note: expanded from macro
> > 'atomic_add_explicit'
> >     In file included from ../lib/dpif-netdev.c:54:
> >     ../lib/netdev-provider.h:99:9: error: address argument to atomic operation
> > must be a pointer to _Atomic type ('uint64_t *' (aka 'unsigned long *') invalid)
> >     ../lib/ovs-atomic-clang.h:82:16: note: expanded from macro
> > 'atomic_add_explicit'
> >     ../lib/netdev.c:2044:5: error: address argument to atomic operation must be
> > a pointer to _Atomic type ('const uint64_t *' (aka 'const unsigned long *')
> > invalid)
> >     ../lib/ovs-atomic-clang.h:53:15: note: expanded from macro
> > 'atomic_read_explicit'
> > 
> > and many more instances.
> > 
> > GCC says:
> > 
> >     ../lib/netdev.c:2044:5: error: incorrect type in argument 1 (different
> > modifiers)
> >     ../lib/netdev.c:2044:5:    expected void *
> >     ../lib/netdev.c:2044:5:    got unsigned long const *
> >     ../lib/netdev.c:2044:5: error: incorrect type in argument 1 (different
> > modifiers)
> >     ../lib/netdev.c:2044:5:    expected void *
> >     ../lib/netdev.c:2044:5:    got unsigned long const *
> > 
> > I do tend to think it's correct, otherwise.  I wonder how this has been missed
> > for so long.
> > 
> > Thanks,
> > 
> > Ben.


More information about the dev mailing list