Thread (16 messages) 16 messages, 4 authors, 2019-03-21

Re: [PATCH v3] Drivers: hv: vmbus: Expose monitor data when channel uses monitor pages

From: Greg KH <gregkh@linuxfoundation.org>
Date: 2019-02-26 08:18:54
Also in: lkml

On Tue, Feb 26, 2019 at 12:35:30AM -0500, Kimberly Brown wrote:
quoted hunk ↗ jump to hunk
There are two methods for signaling the host: the monitor page mechanism
and hypercalls. The monitor page mechanism is used by performance
critical channels (storage, networking, etc.) because it provides
improved throughput. However, latency is increased. Monitor pages are
allocated to these channels.

Monitor pages are not allocated to channels that do not use the monitor
page mechanism. Therefore, these channels do not have a valid monitor id
or valid monitor page data. In these cases, some of the "_show"
functions return incorrect data. They return an invalid monitor id and
data that is beyond the bounds of the hv_monitor_page array fields.

The "channel->offermsg.monitor_allocated" value can be used to determine
whether monitor pages have been allocated to a channel.

Move the device-level monitor page attributes to a separate
attribute_group struct. If the channel uses the monitor page mechanism,
set up the sysfs files for these attributes in vmbus_device_register().

Move the channel-level monitor page attributes to a separate
attribute_group struct. If the channel uses the monitor page mechanism,
set up the sysfs files for these attributes in vmbus_add_channel_kobj().

Signed-off-by: Kimberly Brown <redacted>
---
Changes in v3:
The monitor "_show" functions no longer return an error when a channel
does not use the monitor page mechanism. Instead, the monitor page sysfs
files are created only when a channel uses the monitor page mechanism.
This change was suggested by G. Kroah-Hartman.

Note: this patch was originally patch 2/2 in a patchset. Patch 1/2 has
already been added to char-misc-testing, so I'm not resending it.

Changes in v2:
 - Changed the return value for cases where monitor_allocated is not set
   to "-EINVAL".
 - Updated the commit message to provide more details about the monitor
   page mechanism.
 - Updated the sysfs documentation to describe the new return value.

 Documentation/ABI/stable/sysfs-bus-vmbus | 12 ++++--
 drivers/hv/vmbus_drv.c                   | 52 +++++++++++++++++++-----
 2 files changed, 51 insertions(+), 13 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus
index 826689dcc2e6..6d5cb195b119 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -81,7 +81,9 @@ What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/latency
 Date:		September. 2017
 KernelVersion:	4.14
 Contact:	Stephen Hemminger <sthemmin@microsoft.com>
-Description:	Channel signaling latency
+Description:	Channel signaling latency. This file is available only for
+		performance critical channels (storage, network, etc.) that use
+		the monitor page mechanism.
 Users:		Debugging tools
 
 What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/out_mask
@@ -95,7 +97,9 @@ What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/pending
 Date:		September. 2017
 KernelVersion:	4.14
 Contact:	Stephen Hemminger <sthemmin@microsoft.com>
-Description:	Channel interrupt pending state
+Description:	Channel interrupt pending state. This file is available only for
+                performance critical channels (storage, network, etc.) that use
+                the monitor page mechanism.
 Users:		Debugging tools
 
 What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/read_avail
@@ -137,7 +141,9 @@ What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/monitor_id
 Date:		January. 2018
 KernelVersion:	4.16
 Contact:	Stephen Hemminger <sthemmin@microsoft.com>
-Description:	Monitor bit associated with channel
+Description:	Monitor bit associated with channel. This file is available only
+		for performance critical channels (storage, network, etc.) that
+		use the monitor page mechanism.
 Users:		Debugging tools and userspace drivers
 
 What:		/sys/bus/vmbus/devices/<UUID>/channels/<N>/ring
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 000b53e5a17a..ede858b0ee46 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -601,19 +601,12 @@ static DEVICE_ATTR_RW(driver_override);
 static struct attribute *vmbus_dev_attrs[] = {
 	&dev_attr_id.attr,
 	&dev_attr_state.attr,
-	&dev_attr_monitor_id.attr,
 	&dev_attr_class_id.attr,
 	&dev_attr_device_id.attr,
 	&dev_attr_modalias.attr,
 #ifdef CONFIG_NUMA
 	&dev_attr_numa_node.attr,
 #endif
-	&dev_attr_server_monitor_pending.attr,
-	&dev_attr_client_monitor_pending.attr,
-	&dev_attr_server_monitor_latency.attr,
-	&dev_attr_client_monitor_latency.attr,
-	&dev_attr_server_monitor_conn_id.attr,
-	&dev_attr_client_monitor_conn_id.attr,
 	&dev_attr_out_intr_mask.attr,
 	&dev_attr_out_read_index.attr,
 	&dev_attr_out_write_index.attr,
@@ -632,6 +625,22 @@ static struct attribute *vmbus_dev_attrs[] = {
 };
 ATTRIBUTE_GROUPS(vmbus_dev);
 
+/*
+ *  Set up per device monitor page attributes. These attributes are used only by
+ *  channels that use the monitor page mechanism.
+ */
+static struct attribute *vmbus_dev_monitor_attrs[] = {
+	&dev_attr_monitor_id.attr,
+	&dev_attr_server_monitor_pending.attr,
+	&dev_attr_client_monitor_pending.attr,
+	&dev_attr_server_monitor_latency.attr,
+	&dev_attr_client_monitor_latency.attr,
+	&dev_attr_server_monitor_conn_id.attr,
+	&dev_attr_client_monitor_conn_id.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(vmbus_dev_monitor);
No need to create a special group for this and then manually add it if
you need it.  Just the is_visible() callback in the attribute instead, as
that is what it is there for.

Thanks,

greg k-h
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help