uio_hv_generic: Set event for all channels on the device
PATCH v2 1/1 uio_hv_generic: Set event for all channels on the device
From: Long Li <[email protected]>
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.
---
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(-)
drivers/uio/uio_hv_generic.c +26 -6
@@ -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 (); +vmbus_set_event (channel); + } +} +/* * This is the irqcontrol callback to be registered to uio_info. * It can be used to disable/enable interrupt from user space processes.
@@ -79,12 +89,15 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state) {struct hv_uio_private_data *pdata = info->priv;struct hv_device *dev = pdata->device; +struct vmbus_channel *primary, *sc; - dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state; -virt_mb (); + primary = dev->channel; +set_event (primary, irq_state); -if (!dev->channel->offermsg.monitor_allocated && irq_state) -vmbus_setevent (dev->channel); +mutex_lock (&vmbus_connection.channel_mutex); +list_for_each_entry (sc, &primary->sc_list, sc_list) +set_event (sc, irq_state); +mutex_unlock (&vmbus_connection.channel_mutex);return 0 ; }
@@ -95,12 +108,19 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)static void hv_uio_channel_cb (void *context) {struct vmbus_channel *chan = context; -struct hv_device *hv_dev = chan->device_obj; -struct hv_uio_private_data *pdata =hv_get_drvdata (hv_dev); +struct hv_device *hv_dev; +struct hv_uio_private_data *pdata; chan->inbound.ring_buffer->interrupt_mask =1 ;virt_mb (); +/* +* The callback may come from a subchannel, in which case look +* for the hv device in the primary channel +*/ + hv_dev = chan->primary_channel ? + chan->primary_channel->device_obj : chan->device_obj; + pdata =hv_get_drvdata (hv_dev);uio_event_notify (&pdata->info); }
Signature · 1 line
2.34.1
From: [email protected] <[email protected]> Sent: Friday, February 28, 2025 2:14 PM
Quoted 70 lines
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 <[email protected]>---
Change log
v2: Usevmbus_set_event()to avoid additional check on monitored bit
Lock vmbus_connection.channel_mutex when going through subchannelsdrivers/uio/uio_hv_generic.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)drivers/uio/uio_hv_generic.c +19
-6@@ -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 (); +vmbus_set_event (channel); + } +} +/* * This is the irqcontrol callback to be registered to uio_info. * It can be used to disable/enable interrupt from user space processes. @@ -79,12 +89,15 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state) {struct hv_uio_private_data *pdata = info->priv;struct hv_device *dev = pdata->device; +struct vmbus_channel *primary, *sc; - dev->channel->inbound.ring_buffer->interrupt_mask = !irq_state; -virt_mb (); + primary = dev->channel; +set_event (primary, irq_state); -if (!dev->channel->offermsg.monitor_allocated && irq_state) -vmbus_setevent (dev->channel); +mutex_lock (&vmbus_connection.channel_mutex); +list_for_each_entry (sc, &primary->sc_list, sc_list) +set_event (sc, irq_state); +mutex_unlock (&vmbus_connection.channel_mutex);return 0 ; }@@ -95,12 +108,19 @@ hv_uio_irqcontrol(struct uio_info *info, s32 irq_state)static void hv_uio_channel_cb (void *context) {struct vmbus_channel *chan = context; -struct hv_device *hv_dev = chan->device_obj; -struct hv_uio_private_data *pdata =hv_get_drvdata (hv_dev); +struct hv_device *hv_dev; +struct hv_uio_private_data *pdata; chan->inbound.ring_buffer->interrupt_mask =1 ;virt_mb ();
drivers/uio/uio_hv_generic.c hv_uio_irqcontrol(struct uio_info *info, s32 irq_state) · line 126
Quoted 10 lines
drivers/uio/uio_hv_generic.c +7
-0+/* +* The callback may come from a subchannel, in which case look +* for the hv device in the primary channel +*/ + hv_dev = chan->primary_channel ? + chan->primary_channel->device_obj : chan->device_obj; + pdata =hv_get_drvdata (hv_dev);uio_event_notify (&pdata->info); }
Quoted 23 lines · On Fri, Feb 28, 2025 at 02:14:14PM -0800, [email protected] wrote:
From: Long Li <[email protected]>
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 <[email protected]>---
Change log
v2: Usevmbus_set_event()to avoid additional check on monitored bit
Lock vmbus_connection.channel_mutex when going through subchannelsdrivers/uio/uio_hv_generic.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)drivers/uio/uio_hv_generic.c +0
-0
drivers/uio/uio_hv_generic.c struct hv_uio_private_data { · line 73
Quoted 9 lines
drivers/uio/uio_hv_generic.c +6
-0@@ -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
Quoted 37 lines
Subject: Re: [Patch v2]
uio_hv_generic: Set event for all channels on the deviceQuoted 23 lines · On Fri, Feb 28, 2025 at 02:14:14PM -0800, [email protected] wrote:
From: Long Li <[email protected]>
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 <[email protected]>---
Change log
v2: Usevmbus_set_event()to avoid additional check on monitored bit
Lock vmbus_connection.channel_mutex when going through subchannelsdrivers/uio/uio_hv_generic.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)(quoted diff) +0
-0
b/drivers/uio/uio_hv_generic.cindex3976360d0096..45be2f8baade100644drivers/uio/uio_hv_generic.c struct hv_uio_private_data { · line 72
Quoted 8 lines
drivers/uio/uio_hv_generic.c +5
-0@@ -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 ?
Quoted 1 line
- 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.
Long
Quoted 37 lines · Long Li
Quoted 37 lines
Subject: Re: [Patch v2]
uio_hv_generic: Set event for all channels on the deviceQuoted 23 lines · On Fri, Feb 28, 2025 at 02:14:14PM -0800, [email protected] wrote:
From: Long Li <[email protected]>
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 <[email protected]>---
Change log
v2: Usevmbus_set_event()to avoid additional check on monitored bit
Lock vmbus_connection.channel_mutex when going through subchannelsdrivers/uio/uio_hv_generic.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)(quoted diff) +0
-0
b/drivers/uio/uio_hv_generic.cindex3976360d0096..45be2f8baade100644drivers/uio/uio_hv_generic.c struct hv_uio_private_data { · line 72
Quoted 8 lines
drivers/uio/uio_hv_generic.c +5
-0@@ -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 ?
Quoted 4 lines
Quoted 1 line
- 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) ?
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.
Regards,
Saurabh
Quoted 56 lines
-----Original Message-----
From: Saurabh Singh Sengar <[email protected]>
Sent: Monday, March 10, 2025 10:31 AM
To: Long Li <[email protected]>
Cc: [email protected]; KY Srinivasan <[email protected]>; Haiyang Zhang <[email protected]>; Wei Liu <[email protected]>; Dexuan Cui <[email protected]>; Greg Kroah-Hartman <[email protected]>; [email protected]; [email protected]
Subject: Re: [Patch v2]uio_hv_generic: Set event for all channels on the deviceQuoted 43 lines · Long Li
Quoted 42 lines
Subject: Re: [Patch v2]
uio_hv_generic: Set event for all channels on the deviceQuoted 26 lines · On Fri, Feb 28, 2025 at 02:14:14PM -0800, [email protected] wrote:
From: Long Li <[email protected]>
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 <[email protected]>---
Change log
v2: Usevmbus_set_event()to avoid additional check on monitored bit
Lock vmbus_connection.channel_mutex when going through
subchannelsdrivers/uio/uio_hv_generic.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)(quoted diff) +0
-0
b/drivers/uio/uio_hv_generic.cindex3976360d0096..45be2f8baade100644drivers/uio/uio_hv_generic.c struct hv_uio_private_data { · line 72
Quoted 8 lines
drivers/uio/uio_hv_generic.c +5
-0@@ -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
Quoted 8 lines
Quoted 1 line
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.Quoted 1 line
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().
Quoted 3 lines
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