[EXTERNAL] Re: [Patch v3 6/6] bus/vmbus: set event for channel without monitoring support

[Long Li]

> 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
> longli at 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);
> }

Thank you! You are right it should be RTE_ATOMIC(uint32_t *) not RTE_ATOMIC(uint32_t) *.

I'll send v4.

Long