> Subject: RE: [EXTERNAL] Re: [Patch v3 6/6] bus/vmbus: set event for channel
> without monitoring support
>
> > Subject: [EXTERNAL] Re: [Patch v3 6/6] bus/vmbus: set event for
> > channel without monitoring support
> >
> > On Fri, 4 Apr 2025 17:35:38 -0700
> > lon...@linuxonhyperv.com wrote:
> >
> > > diff --git a/drivers/bus/vmbus/vmbus_channel.c
> > > b/drivers/bus/vmbus/vmbus_channel.c
> > > index bccef168d3..81e8096190 100644
> > > --- a/drivers/bus/vmbus/vmbus_channel.c
> > > +++ b/drivers/bus/vmbus/vmbus_channel.c
> > > @@ -24,6 +24,19 @@ vmbus_sync_set_bit(volatile RTE_ATOMIC(uint32_t)
> > *addr, uint32_t mask)
> > > rte_atomic_fetch_or_explicit(addr, mask,
> > > rte_memory_order_seq_cst); }
> > >
> > > +static inline void
> > > +vmbus_send_interrupt(const struct rte_vmbus_device *dev, uint32_t
> > > +relid) {
> > > + RTE_ATOMIC(uint32_t) *int_addr;
> > > + uint32_t int_mask;
> > > +
> > > + int_addr = (RTE_ATOMIC(uint32_t)*) (dev->int_page + relid / 32);
> > > + int_mask = 1u << (relid % 32);
> > > + vmbus_sync_set_bit(int_addr, int_mask);
> > > +
> > > + vmbus_uio_irq_control(dev, 1);
> > > +}
> > > +
> >
> > This part doesn't look right. RTE_ATOMIC() is just a macro to add the
> > _Atomic attribute.
> >
> > Can it be simplified like this?
> >
> >
> > static inline void
> > vmbus_sync_set_bit(RTE_ATOMIC(uint32_t *) addr, uint32_t mask) {
> > rte_atomic_fetch_or_explicit(addr, mask, rte_memory_order_seq_cst); }
> >
> > static inline void
> > vmbus_send_interrupt(const struct rte_vmbus_device *dev, uint32_t relid) {
> > RTE_ATOMIC(uint32_t *) int_addr;
> > uint32_t int_mask;
> >
> > int_addr = dev->int_page + relid / 32;
> > int_mask = 1u << (relid % 32);
> > vmbus_sync_set_bit(int_addr, int_mask);
> >
> > vmbus_uio_irq_control(dev, 1);
> > }
>
> Hi Stephen,
>
> I need to go back to this version of the patch, as v5 doesn't compile
> successfully
> under clang with the following error:
>
> [459/3553] Compiling C object drivers/libtmp_rte_bus_vmbus.a.p/bus_vmbus
> ccache gcc -Idrivers/libtmp_rte_bus_vmbus.a.p -Idrivers -I../drivers -
> Idrivers/bus/vmbus -I../drivers/bus/vmbus -I../drivers/bus/vmbus/linux -
> Ilib/eal/common -I../lib/eal/common -I. -I.. -Iconfig -I../config
> -Ilib/eal/include -
> I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include -
> Ilib/eal/x86/include -I../lib/eal/x86/include -I../kernel/linux -Ilib/eal
> -I../lib/eal -
> Ilib/kvargs -I../lib/kvargs -Ilib/log -I../lib/log -Ilib/metrics
> -I../lib/metrics -
> Ilib/telemetry -I../lib/telemetry -fdiagnostics-color=always -pipe -
> D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -std=c11 -O2 -g -
> include rte_config.h -Wvla -Wcast-qual -Wdeprecated -Wformat -Wformat-
> nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -
> Wnested-externs -Wold-style-definition -Wpointer-arith -Wsign-compare -
> Wstrict-prototypes -Wundef -Wwrite-strings -Wno-packed-not-aligned -Wno-
> missing-field-initializers -D_GNU_SOURCE -fPIC -march=corei7 -mrtm -
> DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation
> -Wno-address-of-packed-member -DRTE_LOG_DEFAULT_LOGTYPE=bus.vmbus -
> MD -MQ drivers/libtmp_rte_bus_vmbus.a.p/bus_vmbus_vmbus_channel.c.o -MF
> drivers/libtmp_rte_bus_vmbus.a.p/bus_vmbus_vmbus_channel.c.o.d -o
> drivers/libtmp_rte_bus_vmbus.a.p/bus_vmbus_vmbus_channel.c.o -
> c ../drivers/bus/vmbus/vmbus_channel.c
> ../drivers/bus/vmbus/vmbus_channel.c: In function 'vmbus_set_monitor':
> ../drivers/bus/vmbus/vmbus_channel.c:51:22: error: assignment to 'uint32_t *'
> {aka 'unsigned int *'} from incompatible pointer type '_Atomic uint32_t *'
> {aka
> '_Atomic unsigned int *'} [-Werror=incompatible-pointer-types]
> 51 | monitor_addr = &channel->monitor_page-
> >trigs[trigger_index].pending;
> | ^
> cc1: all warnings being treated as errors
>
> I think it's okay to define those pointers as "RTE_ATOMIC(uint32_t) *". The
> pointers themselves are not atomic, but data in the addresses they point to
> are
> atomic.
>
> Do you think it is okay to proceed with this patch?
>
> Thanks,
> Long
I sent out v6 reverting the patch to this version. This compiles okay with
clang and stdatomic flags.
Thanks,
Long
