RE: [Patch v2] uio_hv_generic: Set event for all channels on the device

[Long Li]

> -----Original Message-----
> From: Saurabh Singh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
> Sent: Monday, March 10, 2025 10:31 AM
> To: Long Li <longli@xxxxxxxxxxxxx>
> Cc: longli@xxxxxxxxxxxxxxxxx; KY Srinivasan <kys@xxxxxxxxxxxxx>; Haiyang
> Zhang <haiyangz@xxxxxxxxxxxxx>; Wei Liu <wei.liu@xxxxxxxxxx>; Dexuan Cui
> <decui@xxxxxxxxxxxxx>; Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>;
> linux-hyperv@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [Patch v2] uio_hv_generic: Set event for all channels on the device
> 
> On Mon, Mar 10, 2025 at 05:16:15PM +0000, Long Li wrote:
> > > Subject: Re: [Patch v2] uio_hv_generic: Set event for all channels
> > > on the device
> > >
> > > On Fri, Feb 28, 2025 at 02:14:14PM -0800, longli@xxxxxxxxxxxxxxxxx wrote:
> > > > From: Long Li <longli@xxxxxxxxxxxxx>
> > > >
> > > > Hyper-V may offer a non latency sensitive device with subchannels
> > > > without monitor bit enabled. The decision is entirely on the
> > > > Hyper-V host not configurable within guest.
> > > >
> > > > When a device has subchannels, also signal events for the
> > > > subchannel if its monitor bit is disabled.
> > > >
> > > > Signed-off-by: Long Li <longli@xxxxxxxxxxxxx>
> > > > ---
> > > > Change log
> > > > v2: Use vmbus_set_event() to avoid additional check on monitored bit
> > > >     Lock vmbus_connection.channel_mutex when going through
> > > > subchannels
> > > >
> > > >  drivers/uio/uio_hv_generic.c | 32
> > > > ++++++++++++++++++++++++++------
> > > >  1 file changed, 26 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/uio/uio_hv_generic.c
> > > > b/drivers/uio/uio_hv_generic.c index 3976360d0096..45be2f8baade
> > > > 100644
> > > > --- a/drivers/uio/uio_hv_generic.c
> > > > +++ b/drivers/uio/uio_hv_generic.c
> > > > @@ -65,6 +65,16 @@ struct hv_uio_private_data {
> > > >  	char	send_name[32];
> > > >  };
> > > >
> > > > +static void set_event(struct vmbus_channel *channel, s32 irq_state) {
> > > > +	channel->inbound.ring_buffer->interrupt_mask = !irq_state;
> > > > +	if (!channel->offermsg.monitor_allocated && irq_state) {
> > > > +		/* MB is needed for host to see the interrupt mask first */
> > > > +		virt_mb();
> > >
> > > Why is memory barrier not getting called for 'faster' channels ?
> > >
> > > - Saurabh
> >
> > No, the memory barrier is not needed. Even with a barrier, There is no
> guarantee that all pending IRQs are flushed when hv_uio_irqcontrol() returns. If
> user-mode depends on this guarantee, that user-mode has a bug. This barrier
> adds unnecessary costs when walking through subchannels.
> >
> 
> Thanks for the details. However I didn't understand if memory barrier is not
> required why have it only for 'slow' devices (ie !monitor_allocated) ?

Because memory barrier is needed between setting interrupt mask and calling vmbus_set_event().

> 
> This change also removes the MB for primary channel (if it supports moitor bits), if
> this is intentional, we should call out this in commit message.

Okay, I'm adding it to the commit message.

Long