> 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
