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 <b...@ovn.org>
> Sent: Tuesday, November 26, 2019 5:54 AM
> To: Yanqin Wei (Arm Technology China) <yanqin....@arm.com>
> Cc: d...@openvswitch.org; ovs-dev@openvswitch.org; nd <n...@arm.com>;
> Gavin Hu (Arm Technology China) <gavin...@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...@arm.com>
> > Signed-off-by: Yanqin Wei <yanqin....@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.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to