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. In the affected
"_show" functions, verify that "channel->offermsg.monitor_allocated" is
set before accessing the monitor id or the monitor page data. If
"channel->offermsg.monitor_allocated" is not set, return -EINVAL.
Signed-off-by: Kimberly Brown <[email protected]>
---
Documentation/ABI/stable/sysfs-bus-vmbus | 15 ++++++++--
drivers/hv/vmbus_drv.c | 37 ++++++++++++++++++++++++
2 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus
index 3fed8fdb873d..9e8a7a29b3ff 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -81,7 +81,10 @@ What: /sys/bus/vmbus/devices/<UUID>/channels/<N>/latency
Date: September. 2017
KernelVersion: 4.14
Contact: Stephen Hemminger <[email protected]>
-Description: Channel signaling latency
+Description: Channel signaling latency. The monitor page mechanism is used
+ for performance critical channels (storage, network, etc.).
+ Channels that do not use the monitor page mechanism will return
+ EINVAL.
Users: Debugging tools
What: /sys/bus/vmbus/devices/<UUID>/channels/<N>/out_mask
@@ -95,7 +98,10 @@ What: /sys/bus/vmbus/devices/<UUID>/channels/<N>/pending
Date: September. 2017
KernelVersion: 4.14
Contact: Stephen Hemminger <[email protected]>
-Description: Channel interrupt pending state
+Description: Channel interrupt pending state. The monitor page mechanism is
+ used for performance critical channels (storage, network,
+ etc.). Channels that do not use the monitor page mechanism will
+ return EINVAL.
Users: Debugging tools
What: /sys/bus/vmbus/devices/<UUID>/channels/<N>/read_avail
@@ -137,7 +143,10 @@ What: /sys/bus/vmbus/devices/<UUID>/channels/<N>/monitor_id
Date: January. 2018
KernelVersion: 4.16
Contact: Stephen Hemminger <[email protected]>
-Description: Monitor bit associated with channel
+Description: Monitor bit associated with channel. The monitor page mechanism
+ is used for performance critical channels (storage, network,
+ etc.). Channels that do not use the monitor page mechanism will
+ return EINVAL.
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 f2a79f5129d7..0ff9a09a3f71 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -171,6 +171,10 @@ static ssize_t monitor_id_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+
+ if (!hv_dev->channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%d\n", hv_dev->channel->offermsg.monitorid);
}
static DEVICE_ATTR_RO(monitor_id);
@@ -232,6 +236,10 @@ static ssize_t server_monitor_pending_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+
+ if (!hv_dev->channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%d\n",
channel_pending(hv_dev->channel,
vmbus_connection.monitor_pages[0]));
@@ -246,6 +254,10 @@ static ssize_t client_monitor_pending_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+
+ if (!hv_dev->channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%d\n",
channel_pending(hv_dev->channel,
vmbus_connection.monitor_pages[1]));
@@ -260,6 +272,10 @@ static ssize_t server_monitor_latency_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+
+ if (!hv_dev->channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%d\n",
channel_latency(hv_dev->channel,
vmbus_connection.monitor_pages[0]));
@@ -274,6 +290,10 @@ static ssize_t client_monitor_latency_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+
+ if (!hv_dev->channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%d\n",
channel_latency(hv_dev->channel,
vmbus_connection.monitor_pages[1]));
@@ -288,6 +308,10 @@ static ssize_t server_monitor_conn_id_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+
+ if (!hv_dev->channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%d\n",
channel_conn_id(hv_dev->channel,
vmbus_connection.monitor_pages[0]));
@@ -302,6 +326,10 @@ static ssize_t client_monitor_conn_id_show(struct device *dev,
if (!hv_dev->channel)
return -ENODEV;
+
+ if (!hv_dev->channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%d\n",
channel_conn_id(hv_dev->channel,
vmbus_connection.monitor_pages[1]));
@@ -1469,6 +1497,9 @@ static VMBUS_CHAN_ATTR(cpu, S_IRUGO, show_target_cpu, NULL);
static ssize_t channel_pending_show(const struct vmbus_channel *channel,
char *buf)
{
+ if (!channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%d\n",
channel_pending(channel,
vmbus_connection.monitor_pages[1]));
@@ -1478,6 +1509,9 @@ static VMBUS_CHAN_ATTR(pending, S_IRUGO, channel_pending_show, NULL);
static ssize_t channel_latency_show(const struct vmbus_channel *channel,
char *buf)
{
+ if (!channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%d\n",
channel_latency(channel,
vmbus_connection.monitor_pages[1]));
@@ -1499,6 +1533,9 @@ static VMBUS_CHAN_ATTR(events, S_IRUGO, channel_events_show, NULL);
static ssize_t subchannel_monitor_id_show(const struct vmbus_channel *channel,
char *buf)
{
+ if (!channel->offermsg.monitor_allocated)
+ return -EINVAL;
+
return sprintf(buf, "%u\n", channel->offermsg.monitorid);
}
static VMBUS_CHAN_ATTR(monitor_id, S_IRUGO, subchannel_monitor_id_show, NULL);
--
2.17.1
From: Kimberly Brown <[email protected]> Sent: Monday, February 18, 2019 9:38 PM
>
> 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. In the affected
> "_show" functions, verify that "channel->offermsg.monitor_allocated" is
> set before accessing the monitor id or the monitor page data. If
> "channel->offermsg.monitor_allocated" is not set, return -EINVAL.
>
> Signed-off-by: Kimberly Brown <[email protected]>
>
Reviewed-by: Michael Kelley <[email protected]>
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 <[email protected]>
---
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 <[email protected]>
-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 <[email protected]>
-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 <[email protected]>
-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);
+
/*
* vmbus_uevent - add uevent for our device
*
@@ -1537,19 +1546,30 @@ static struct attribute *vmbus_chan_attrs[] = {
&chan_attr_read_avail.attr,
&chan_attr_write_avail.attr,
&chan_attr_cpu.attr,
- &chan_attr_pending.attr,
- &chan_attr_latency.attr,
&chan_attr_interrupts.attr,
&chan_attr_events.attr,
&chan_attr_intr_in_full.attr,
&chan_attr_intr_out_empty.attr,
&chan_attr_out_full_first.attr,
&chan_attr_out_full_total.attr,
- &chan_attr_monitor_id.attr,
&chan_attr_subchannel_id.attr,
NULL
};
+/* Set up per channel monitor page attributes. These attributes are used only by
+ * channels that use the monitor page mechanism.
+ */
+static struct attribute *vmbus_chan_monitor_attrs[] = {
+ &chan_attr_pending.attr,
+ &chan_attr_latency.attr,
+ &chan_attr_monitor_id.attr,
+ NULL
+};
+
+static struct attribute_group vmbus_chan_monitor_attr_group = {
+ .attrs = vmbus_chan_monitor_attrs,
+};
+
static struct kobj_type vmbus_chan_ktype = {
.sysfs_ops = &vmbus_chan_sysfs_ops,
.release = vmbus_chan_release,
@@ -1571,6 +1591,15 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
if (ret)
return ret;
+ if (channel->offermsg.monitor_allocated) {
+ ret = sysfs_create_group(kobj, &vmbus_chan_monitor_attr_group);
+ if (ret) {
+ pr_err("Unable to set up monitor page sysfs files");
+ kobject_put(kobj);
+ return ret;
+ }
+ }
+
kobject_uevent(kobj, KOBJ_ADD);
return 0;
@@ -1615,6 +1644,9 @@ int vmbus_device_register(struct hv_device *child_device_obj)
child_device_obj->device.parent = &hv_acpi_dev->dev;
child_device_obj->device.release = vmbus_device_release;
+ if (child_device_obj->channel->offermsg.monitor_allocated)
+ child_device_obj->device.groups = vmbus_dev_monitor_groups;
+
/*
* Register with the LDM. This will kick off the driver/device
* binding...which will eventually call vmbus_match() and vmbus_probe()
--
2.17.1
On Tue, Feb 26, 2019 at 12:35:30AM -0500, Kimberly Brown wrote:
> 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 <[email protected]>
> ---
> 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 <[email protected]>
> -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 <[email protected]>
> -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 <[email protected]>
> -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
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.
Add "is_visible()" callback functions for the device-level and
channel-level attribute groups. These functions will hide the monitor
sysfs files when the monitor mechanism is not used.
Remove ".default_attributes" from "vmbus_chan_attrs" and create a
channel-level attribute group, "vmbus_chan_group". These changes allow
the new "is_visible()" callback function to be applied to the
channel-level attributes. Call "sysfs_create_group()" to create the
channel sysyfs files.
Signed-off-by: Kimberly Brown <[email protected]>
---
Changes in v4:
- Added “is_visible()” callback functions for the device-level and
channel-level attribute groups.
- Removed the separate monitor attribute groups proposed in v3. They’re
no longer needed because the “is_visible()” callbacks are used to
determine the attribute visibility.
- Removed "default_attributes" from "vmbus_chan_attrs" and created a
channel-level attribute group.
- Removed the "kobject_put(kobj)" call proposed in v3. The calling
functions take care of calling "kobject_put(channel->kobj)" if an
error is returned by "vmbus_add_channel_kobj()".
- Updated the commit message and subject for clarity and to reflect the
new changes in v4.
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 | 63 +++++++++++++++++++++++-
2 files changed, 70 insertions(+), 5 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus
index 826689dcc2e6..8e8d167eca31 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 <[email protected]>
-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 <[email protected]>
-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 <[email protected]>
-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..44308220d540 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -630,7 +630,36 @@ static struct attribute *vmbus_dev_attrs[] = {
&dev_attr_driver_override.attr,
NULL,
};
-ATTRIBUTE_GROUPS(vmbus_dev);
+
+/*
+ * Device-level attribute_group callback function. Returns the permission for
+ * each attribute, and returns 0 if an attribute is not visible.
+ */
+static umode_t vmbus_dev_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ const struct hv_device *hv_dev = device_to_hv_device(dev);
+
+ /* Hide the monitor attributes if the monitor mechanism is not used. */
+ if (!hv_dev->channel->offermsg.monitor_allocated &&
+ (attr == &dev_attr_monitor_id.attr ||
+ attr == &dev_attr_server_monitor_pending.attr ||
+ attr == &dev_attr_client_monitor_pending.attr ||
+ attr == &dev_attr_server_monitor_latency.attr ||
+ attr == &dev_attr_client_monitor_latency.attr ||
+ attr == &dev_attr_server_monitor_conn_id.attr ||
+ attr == &dev_attr_client_monitor_conn_id.attr))
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group vmbus_dev_group = {
+ .attrs = vmbus_dev_attrs,
+ .is_visible = vmbus_dev_attr_is_visible
+};
+__ATTRIBUTE_GROUPS(vmbus_dev);
/*
* vmbus_uevent - add uevent for our device
@@ -1550,10 +1579,34 @@ static struct attribute *vmbus_chan_attrs[] = {
NULL
};
+/*
+ * Channel-level attribute_group callback function. Returns the permission for
+ * each attribute, and returns 0 if an attribute is not visible.
+ */
+static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ const struct vmbus_channel *channel =
+ container_of(kobj, struct vmbus_channel, kobj);
+
+ /* Hide the monitor attributes if the monitor mechanism is not used. */
+ if (!channel->offermsg.monitor_allocated &&
+ (attr == &chan_attr_pending.attr ||
+ attr == &chan_attr_latency.attr ||
+ attr == &chan_attr_monitor_id.attr))
+ return 0;
+
+ return attr->mode;
+}
+
+static struct attribute_group vmbus_chan_group = {
+ .attrs = vmbus_chan_attrs,
+ .is_visible = vmbus_chan_attr_is_visible
+};
+
static struct kobj_type vmbus_chan_ktype = {
.sysfs_ops = &vmbus_chan_sysfs_ops,
.release = vmbus_chan_release,
- .default_attrs = vmbus_chan_attrs,
};
/*
@@ -1571,6 +1624,12 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
if (ret)
return ret;
+ ret = sysfs_create_group(kobj, &vmbus_chan_group);
+ if (ret) {
+ pr_err("Unable to set up channel sysfs files\n");
+ return ret;
+ }
+
kobject_uevent(kobj, KOBJ_ADD);
return 0;
--
2.17.1
From: Kimberly Brown <[email protected]> Sent: Friday, March 1, 2019 11:18 AM
>
> +/*
> + * Channel-level attribute_group callback function. Returns the permission for
> + * each attribute, and returns 0 if an attribute is not visible.
> + */
> +static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int idx)
> +{
> + const struct vmbus_channel *channel =
> + container_of(kobj, struct vmbus_channel, kobj);
> +
> + /* Hide the monitor attributes if the monitor mechanism is not used. */
> + if (!channel->offermsg.monitor_allocated &&
> + (attr == &chan_attr_pending.attr ||
> + attr == &chan_attr_latency.attr ||
> + attr == &chan_attr_monitor_id.attr))
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static struct attribute_group vmbus_chan_group = {
> + .attrs = vmbus_chan_attrs,
> + .is_visible = vmbus_chan_attr_is_visible
> +};
> +
> static struct kobj_type vmbus_chan_ktype = {
> .sysfs_ops = &vmbus_chan_sysfs_ops,
> .release = vmbus_chan_release,
> - .default_attrs = vmbus_chan_attrs,
Just to double-check my understanding, removing the default_attrs
here means that in vmbus_add_channel_kobj(), the call to
kobject_init_and_add() will only create the sub-directory that is the
channel number. The sub-directory will be empty. Then the new
call to sysfs_create_group() that you added below will populate
the subdirectory, as filtered by vmbus_chan_attr_is_visible().
> };
>
> /*
> @@ -1571,6 +1624,12 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct
> vmbus_channel *channel)
> if (ret)
> return ret;
>
> + ret = sysfs_create_group(kobj, &vmbus_chan_group);
> + if (ret) {
> + pr_err("Unable to set up channel sysfs files\n");
> + return ret;
In this error case, the previously created sub-directory that is the
channel number needs to be deleted/cleaned up.
> + }
> +
Michael
On Fri, Mar 01, 2019 at 02:18:24PM -0500, Kimberly Brown wrote:
> +/*
> + * Channel-level attribute_group callback function. Returns the permission for
> + * each attribute, and returns 0 if an attribute is not visible.
> + */
> +static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
> + struct attribute *attr, int idx)
> +{
> + const struct vmbus_channel *channel =
> + container_of(kobj, struct vmbus_channel, kobj);
> +
> + /* Hide the monitor attributes if the monitor mechanism is not used. */
> + if (!channel->offermsg.monitor_allocated &&
> + (attr == &chan_attr_pending.attr ||
> + attr == &chan_attr_latency.attr ||
> + attr == &chan_attr_monitor_id.attr))
> + return 0;
> +
> + return attr->mode;
> +}
> +
> +static struct attribute_group vmbus_chan_group = {
> + .attrs = vmbus_chan_attrs,
> + .is_visible = vmbus_chan_attr_is_visible
> +};
> +
> static struct kobj_type vmbus_chan_ktype = {
> .sysfs_ops = &vmbus_chan_sysfs_ops,
> .release = vmbus_chan_release,
> - .default_attrs = vmbus_chan_attrs,
Why did you remove this line?
> };
>
> /*
> @@ -1571,6 +1624,12 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
> if (ret)
> return ret;
>
> + ret = sysfs_create_group(kobj, &vmbus_chan_group);
Why are you adding these "by hand"? What was wrong with using the
default attribute group pointer? You also are not removing the
attributes :(
greg k-h
On Sun, Mar 03, 2019 at 09:05:43AM +0100, Greg KH wrote:
> On Fri, Mar 01, 2019 at 02:18:24PM -0500, Kimberly Brown wrote:
> > +/*
> > + * Channel-level attribute_group callback function. Returns the permission for
> > + * each attribute, and returns 0 if an attribute is not visible.
> > + */
> > +static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
> > + struct attribute *attr, int idx)
> > +{
> > + const struct vmbus_channel *channel =
> > + container_of(kobj, struct vmbus_channel, kobj);
> > +
> > + /* Hide the monitor attributes if the monitor mechanism is not used. */
> > + if (!channel->offermsg.monitor_allocated &&
> > + (attr == &chan_attr_pending.attr ||
> > + attr == &chan_attr_latency.attr ||
> > + attr == &chan_attr_monitor_id.attr))
> > + return 0;
> > +
> > + return attr->mode;
> > +}
> > +
> > +static struct attribute_group vmbus_chan_group = {
> > + .attrs = vmbus_chan_attrs,
> > + .is_visible = vmbus_chan_attr_is_visible
> > +};
> > +
> > static struct kobj_type vmbus_chan_ktype = {
> > .sysfs_ops = &vmbus_chan_sysfs_ops,
> > .release = vmbus_chan_release,
> > - .default_attrs = vmbus_chan_attrs,
>
> Why did you remove this line?
I removed the default attributes because vmbus_chan_attrs contains
non-default attributes. You suggested that I use one attribute_group and
an is_visible() callback for the device-level attributes (see
https://lore.kernel.org/lkml/[email protected]/). I
assumed (possibly incorrectly) that I should do the same for these
channel-level attributes.
> > };
> >
> > /*
> > @@ -1571,6 +1624,12 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
> > if (ret)
> > return ret;
> >
> > + ret = sysfs_create_group(kobj, &vmbus_chan_group);
>
> Why are you adding these "by hand"? What was wrong with using the
> default attribute group pointer? You also are not removing the
> attributes :(
Are you referring to default_attrs in kobj_type? It's not an
attribute_group pointer, it's a pointer to an attribute pointer array.
The problem with using default_attrs is that all of the attributes are
visible.
I'm fairly certain that the monitor attributes are being removed.
sysfs_create_group() uses the attribute_group's is_visible() callback to
control the attribute visibility. And, when I look at the sysfs files, I
can see that the monitor sysyfs files are removed.
In v3, I proposed moving the monitor attributes to a special
attribute_group and adding that group manually when needed. Do you
prefer that approach for the channel-level monitor attributes?
Thanks,
Kim
>
> greg k-h
On Sat, Mar 02, 2019 at 06:39:30PM +0000, Michael Kelley wrote:
> From: Kimberly Brown <[email protected]> Sent: Friday, March 1, 2019 11:18 AM
> >
> > +/*
> > + * Channel-level attribute_group callback function. Returns the permission for
> > + * each attribute, and returns 0 if an attribute is not visible.
> > + */
> > +static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
> > + struct attribute *attr, int idx)
> > +{
> > + const struct vmbus_channel *channel =
> > + container_of(kobj, struct vmbus_channel, kobj);
> > +
> > + /* Hide the monitor attributes if the monitor mechanism is not used. */
> > + if (!channel->offermsg.monitor_allocated &&
> > + (attr == &chan_attr_pending.attr ||
> > + attr == &chan_attr_latency.attr ||
> > + attr == &chan_attr_monitor_id.attr))
> > + return 0;
> > +
> > + return attr->mode;
> > +}
> > +
> > +static struct attribute_group vmbus_chan_group = {
> > + .attrs = vmbus_chan_attrs,
> > + .is_visible = vmbus_chan_attr_is_visible
> > +};
> > +
> > static struct kobj_type vmbus_chan_ktype = {
> > .sysfs_ops = &vmbus_chan_sysfs_ops,
> > .release = vmbus_chan_release,
> > - .default_attrs = vmbus_chan_attrs,
>
> Just to double-check my understanding, removing the default_attrs
> here means that in vmbus_add_channel_kobj(), the call to
> kobject_init_and_add() will only create the sub-directory that is the
> channel number. The sub-directory will be empty. Then the new
> call to sysfs_create_group() that you added below will populate
> the subdirectory, as filtered by vmbus_chan_attr_is_visible().
Yes, that's my understanding as well.
>
> > };
> >
> > /*
> > @@ -1571,6 +1624,12 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct
> > vmbus_channel *channel)
> > if (ret)
> > return ret;
> >
> > + ret = sysfs_create_group(kobj, &vmbus_chan_group);
> > + if (ret) {
> > + pr_err("Unable to set up channel sysfs files\n");
> > + return ret;
>
> In this error case, the previously created sub-directory that is the
> channel number needs to be deleted/cleaned up.
I was going to let the kobject and sysfs systems take care of that. If
vmbus_add_channel_kobj() returns an error to the calling functions, they
call kobject_put(), which eventually removes the directory.
There are two functions that call vmbus_add_channel_kobj():
vmbus_add_channel_work() and vmbus_device_register(). If
vmbus_add_channel_kobj() returns an error to vmbus_add_channel_work(),
the function calls are:
free_channel() => kobject_put() (channel kobj ref. count should now be
0) => kobject_release() => kobject_cleanup() => kobject_del() =>
sysfs_remove_dir()
If vmbus_add_channel_kobject() returns an error to
vmbus_device_register(), the device's sub-directory is removed:
device_unregister() => put_device() => kobject_put() (device kobj ref.
count should now be 0) => kobject_release() => ...
So, I don't think its necessary to cleanup the subdirectory created by
kobject_init_and_add() here.
Thanks,
Kim
>
> > + }
> > +
>
> Michael
On Sun, Mar 03, 2019 at 04:11:28PM -0500, Kimberly Brown wrote:
> On Sun, Mar 03, 2019 at 09:05:43AM +0100, Greg KH wrote:
> > On Fri, Mar 01, 2019 at 02:18:24PM -0500, Kimberly Brown wrote:
> > > +/*
> > > + * Channel-level attribute_group callback function. Returns the permission for
> > > + * each attribute, and returns 0 if an attribute is not visible.
> > > + */
> > > +static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
> > > + struct attribute *attr, int idx)
> > > +{
> > > + const struct vmbus_channel *channel =
> > > + container_of(kobj, struct vmbus_channel, kobj);
> > > +
> > > + /* Hide the monitor attributes if the monitor mechanism is not used. */
> > > + if (!channel->offermsg.monitor_allocated &&
> > > + (attr == &chan_attr_pending.attr ||
> > > + attr == &chan_attr_latency.attr ||
> > > + attr == &chan_attr_monitor_id.attr))
> > > + return 0;
> > > +
> > > + return attr->mode;
> > > +}
> > > +
> > > +static struct attribute_group vmbus_chan_group = {
> > > + .attrs = vmbus_chan_attrs,
> > > + .is_visible = vmbus_chan_attr_is_visible
> > > +};
> > > +
> > > static struct kobj_type vmbus_chan_ktype = {
> > > .sysfs_ops = &vmbus_chan_sysfs_ops,
> > > .release = vmbus_chan_release,
> > > - .default_attrs = vmbus_chan_attrs,
> >
> > Why did you remove this line?
>
> I removed the default attributes because vmbus_chan_attrs contains
> non-default attributes. You suggested that I use one attribute_group and
> an is_visible() callback for the device-level attributes (see
> https://lore.kernel.org/lkml/[email protected]/). I
> assumed (possibly incorrectly) that I should do the same for these
> channel-level attributes.
That is fine to have a callback, but why did you have to remove the
default attribute pointer? The two should have nothing to do with each
other.
> > > };
> > >
> > > /*
> > > @@ -1571,6 +1624,12 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
> > > if (ret)
> > > return ret;
> > >
> > > + ret = sysfs_create_group(kobj, &vmbus_chan_group);
> >
> > Why are you adding these "by hand"? What was wrong with using the
> > default attribute group pointer? You also are not removing the
> > attributes :(
>
> Are you referring to default_attrs in kobj_type? It's not an
> attribute_group pointer, it's a pointer to an attribute pointer array.
> The problem with using default_attrs is that all of the attributes are
> visible.
It shouldn't, the is_visable() callback will be called on each attribute
when the group is created by the core. Did that not work properly when
you tested this?
> I'm fairly certain that the monitor attributes are being removed.
> sysfs_create_group() uses the attribute_group's is_visible() callback to
> control the attribute visibility. And, when I look at the sysfs files, I
> can see that the monitor sysyfs files are removed.
I mean you are not removing the group when the device goes away, not
that the individual files are not present. If you leave the pointer to
default_attributes there, the core will properly remove the sysfs
attributes when the device is removed from the system. Otherwise you
just "get lucky" if the attributes are removed or not.
> In v3, I proposed moving the monitor attributes to a special
> attribute_group and adding that group manually when needed. Do you
> prefer that approach for the channel-level monitor attributes?
No need for a special group here, just use the is_visable() callback
like you currently are, all should be fine. I think you are adding more
code than you need to in order to get this to all work properly :)
thanks,
greg k-h
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.
Add "is_visible()" callback functions for the device-level and
channel-level attribute groups. These functions will hide the monitor
sysfs files when the monitor mechanism is not used.
Remove ".default_attributes" from "vmbus_chan_attrs" and create a
channel-level attribute group, "vmbus_chan_group". These changes allow
the new "is_visible()" callback function to be applied to the
channel-level attributes. Call "sysfs_create_group()" to create the
channel sysyfs files.
Add the “bool sysfs_group_ready” field to the vmbus_channel struct.
This field is used to ensure that the attributes in the
“vmbus_chan_group” attribute group have been created. Add the
“vmbus_remove_channel_attr_group()” function, which calls
"sysfs_remove_group()" on “vmbus_chan_group” if the attributes were
created.
Signed-off-by: Kimberly Brown <[email protected]>
---
Changes in v5:
- Added the “bool sysfs_group_ready” field to vmbus_channel.
- Added the “vmbus_remove_channel_attr_group()” function which calls
"sysfs_remove_group()".
- Added a comment to "vmbus_add_channel_kobj()" to describe how the
empty directory is removed if "sysfs_create_group()" returns an
error.
- Updated the commit message.
NOTE: “.default_attrs” must be removed from vmbus_chan_ktype in order to
use the is_visible() function because "default_attrs" is an array of
attributes, not an attribute_group.
Changes in v4:
- Added “is_visible()” callback functions for the device-level and
channel-level attribute groups.
- Removed the separate monitor attribute groups proposed in v3. They’re
no longer needed because the “is_visible()” callbacks are used to
determine the attribute visibility.
- Removed "default_attributes" from "vmbus_chan_attrs" and created a
channel-level attribute group.
- Removed the "kobject_put(kobj)" call proposed in v3. The calling
functions take care of calling "kobject_put(channel->kobj)" if an
error is returned by "vmbus_add_channel_kobj()".
- Updated the commit message and subject for clarity and to reflect the
new changes in v4.
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/channel_mgmt.c | 3 +
drivers/hv/hyperv_vmbus.h | 2 +
drivers/hv/vmbus_drv.c | 80 +++++++++++++++++++++++-
include/linux/hyperv.h | 6 ++
5 files changed, 98 insertions(+), 5 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus
index 826689dcc2e6..8e8d167eca31 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 <[email protected]>
-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 <[email protected]>
-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 <[email protected]>
-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/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 62703b354d6d..e4b1f0db8159 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -346,6 +346,9 @@ static void free_channel(struct vmbus_channel *channel)
{
tasklet_kill(&channel->callback_event);
+ /* Remove the channel sysfs attribute group */
+ vmbus_remove_channel_attr_group(channel);
+
kobject_put(&channel->kobj);
}
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index cb86b133eb4d..a94aab94e0b5 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -321,6 +321,8 @@ void vmbus_device_unregister(struct hv_device *device_obj);
int vmbus_add_channel_kobj(struct hv_device *device_obj,
struct vmbus_channel *channel);
+void vmbus_remove_channel_attr_group(struct vmbus_channel *channel);
+
struct vmbus_channel *relid2channel(u32 relid);
void vmbus_free_channels(void);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 000b53e5a17a..43a17e17f670 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -630,7 +630,36 @@ static struct attribute *vmbus_dev_attrs[] = {
&dev_attr_driver_override.attr,
NULL,
};
-ATTRIBUTE_GROUPS(vmbus_dev);
+
+/*
+ * Device-level attribute_group callback function. Returns the permission for
+ * each attribute, and returns 0 if an attribute is not visible.
+ */
+static umode_t vmbus_dev_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ const struct hv_device *hv_dev = device_to_hv_device(dev);
+
+ /* Hide the monitor attributes if the monitor mechanism is not used. */
+ if (!hv_dev->channel->offermsg.monitor_allocated &&
+ (attr == &dev_attr_monitor_id.attr ||
+ attr == &dev_attr_server_monitor_pending.attr ||
+ attr == &dev_attr_client_monitor_pending.attr ||
+ attr == &dev_attr_server_monitor_latency.attr ||
+ attr == &dev_attr_client_monitor_latency.attr ||
+ attr == &dev_attr_server_monitor_conn_id.attr ||
+ attr == &dev_attr_client_monitor_conn_id.attr))
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group vmbus_dev_group = {
+ .attrs = vmbus_dev_attrs,
+ .is_visible = vmbus_dev_attr_is_visible
+};
+__ATTRIBUTE_GROUPS(vmbus_dev);
/*
* vmbus_uevent - add uevent for our device
@@ -1550,10 +1579,34 @@ static struct attribute *vmbus_chan_attrs[] = {
NULL
};
+/*
+ * Channel-level attribute_group callback function. Returns the permission for
+ * each attribute, and returns 0 if an attribute is not visible.
+ */
+static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ const struct vmbus_channel *channel =
+ container_of(kobj, struct vmbus_channel, kobj);
+
+ /* Hide the monitor attributes if the monitor mechanism is not used. */
+ if (!channel->offermsg.monitor_allocated &&
+ (attr == &chan_attr_pending.attr ||
+ attr == &chan_attr_latency.attr ||
+ attr == &chan_attr_monitor_id.attr))
+ return 0;
+
+ return attr->mode;
+}
+
+static struct attribute_group vmbus_chan_group = {
+ .attrs = vmbus_chan_attrs,
+ .is_visible = vmbus_chan_attr_is_visible
+};
+
static struct kobj_type vmbus_chan_ktype = {
.sysfs_ops = &vmbus_chan_sysfs_ops,
.release = vmbus_chan_release,
- .default_attrs = vmbus_chan_attrs,
};
/*
@@ -1571,11 +1624,34 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
if (ret)
return ret;
+ ret = sysfs_create_group(kobj, &vmbus_chan_group);
+ channel->sysfs_group_ready = !ret;
+
+ if (ret) {
+ /*
+ * If an error is returned to the calling functions, those
+ * functions will call kobject_put() on the channel kobject,
+ * which will cleanup the empty channel directory created by
+ * kobject_init_and_add().
+ */
+ pr_err("Unable to set up channel sysfs files\n");
+ return ret;
+ }
+
kobject_uevent(kobj, KOBJ_ADD);
return 0;
}
+/*
+ * vmbus_remove_channel_attr_group - remove the channel's attribute group
+ */
+void vmbus_remove_channel_attr_group(struct vmbus_channel *channel)
+{
+ if (channel->sysfs_group_ready)
+ sysfs_remove_group(&channel->kobj, &vmbus_chan_group);
+}
+
/*
* vmbus_device_create - Creates and registers a new child device
* on the vmbus.
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 64698ec8f2ac..604a2e05af47 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -934,6 +934,12 @@ struct vmbus_channel {
* full outbound ring buffer.
*/
u64 out_full_first;
+
+ /*
+ * Indicates whether the channel's attribute group sysfs files have
+ * been successfully created.
+ */
+ bool sysfs_group_ready;
};
static inline bool is_hvsock_channel(const struct vmbus_channel *c)
--
2.17.1
On Fri, Mar 08, 2019 at 05:46:11PM -0500, Kimberly Brown wrote:
> static struct kobj_type vmbus_chan_ktype = {
> .sysfs_ops = &vmbus_chan_sysfs_ops,
> .release = vmbus_chan_release,
> - .default_attrs = vmbus_chan_attrs,
As discussed on IRC, a kobj_type needs to get an attribute group one of
these days, not just a attribute list :)
So thanks for persisting with this, sorry for the earlier review
comments, you were totally right about this, nice work.
Minor review comments below:
> };
>
> /*
> @@ -1571,11 +1624,34 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
> if (ret)
> return ret;
>
> + ret = sysfs_create_group(kobj, &vmbus_chan_group);
> + channel->sysfs_group_ready = !ret;
Why do you need this flag?
> +
> + if (ret) {
> + /*
> + * If an error is returned to the calling functions, those
> + * functions will call kobject_put() on the channel kobject,
> + * which will cleanup the empty channel directory created by
> + * kobject_init_and_add().
Why is this comment needed?
> + */
> + pr_err("Unable to set up channel sysfs files\n");
dev_err() to show who had the problem with the files?
> + return ret;
> + }
> +
> kobject_uevent(kobj, KOBJ_ADD);
>
> return 0;
> }
>
> +/*
> + * vmbus_remove_channel_attr_group - remove the channel's attribute group
> + */
> +void vmbus_remove_channel_attr_group(struct vmbus_channel *channel)
> +{
> + if (channel->sysfs_group_ready)
> + sysfs_remove_group(&channel->kobj, &vmbus_chan_group);
You should be able to just always remove these, no need for a flag to
say you have created them or not, right?
thanks,
greg k-h
On Sat, Mar 09, 2019 at 08:21:35AM +0100, Greg KH wrote:
> On Fri, Mar 08, 2019 at 05:46:11PM -0500, Kimberly Brown wrote:
> > static struct kobj_type vmbus_chan_ktype = {
> > .sysfs_ops = &vmbus_chan_sysfs_ops,
> > .release = vmbus_chan_release,
> > - .default_attrs = vmbus_chan_attrs,
>
> As discussed on IRC, a kobj_type needs to get an attribute group one of
> these days, not just a attribute list :)
>
> So thanks for persisting with this, sorry for the earlier review
> comments, you were totally right about this, nice work.
>
> Minor review comments below:
>
> > };
> >
> > /*
> > @@ -1571,11 +1624,34 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
> > if (ret)
> > return ret;
> >
> > + ret = sysfs_create_group(kobj, &vmbus_chan_group);
> > + channel->sysfs_group_ready = !ret;
>
> Why do you need this flag?
>
I added this flag to prevent sysfs_remove_group() from being called if
sysfs_create_group() or the earlier call to kobject_init_and_add()
failed. However, it looks like that would just lead to WARN() being
called, which seems appropriate. I'll remove this flag.
> > +
> > + if (ret) {
> > + /*
> > + * If an error is returned to the calling functions, those
> > + * functions will call kobject_put() on the channel kobject,
> > + * which will cleanup the empty channel directory created by
> > + * kobject_init_and_add().
>
> Why is this comment needed?
>
Another reviewer asked why the empty directory created by
kobject_init_and_add() isn't removed here when sysfs_create_group()
fails. We decided that a comment would help clear up any future
confusion.
> > + */
> > + pr_err("Unable to set up channel sysfs files\n");
>
> dev_err() to show who had the problem with the files?
>
Yes, good point. I'll change this.
> > + return ret;
> > + }
> > +
> > kobject_uevent(kobj, KOBJ_ADD);
> >
> > return 0;
> > }
> >
> > +/*
> > + * vmbus_remove_channel_attr_group - remove the channel's attribute group
> > + */
> > +void vmbus_remove_channel_attr_group(struct vmbus_channel *channel)
> > +{
> > + if (channel->sysfs_group_ready)
> > + sysfs_remove_group(&channel->kobj, &vmbus_chan_group);
>
> You should be able to just always remove these, no need for a flag to
> say you have created them or not, right?
>
> thanks,
>
> greg k-h
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.
Add "is_visible()" callback functions for the device-level and
channel-level attribute groups. These functions will hide the monitor
sysfs files when the monitor mechanism is not used.
Remove ".default_attributes" from "vmbus_chan_attrs" and create a
channel-level attribute group. These changes allow the new
"is_visible()" callback function to be applied to the channel-level
attributes.
Call "sysfs_create_group()" in "vmbus_add_channel_kobj()" to create the
channel's sysfs files. Add a new function,
“vmbus_remove_channel_attr_group()”, and call it in "free_channel()" to
remove the channel's sysfs files when the channel is closed.
Signed-off-by: Kimberly Brown <[email protected]>
---
Changes in v6:
- Removed the “sysfs_group_ready” flag proposed in v5. The flag was
used to verify that the attribute group was created, which is
unnecessary; sysfs_remove_group() can be called even when
sysfs_create_group() fails.
- Replaced pr_err() with dev_err() as suggested by G. Kroah-Hartman.
- Shortened the error path comment in vmbus_add_channel_kobj().
- Updated the commit message.
Changes in v5:
- Added the “bool sysfs_group_ready” field to vmbus_channel.
- Added the “vmbus_remove_channel_attr_group()” function which calls
"sysfs_remove_group()".
- Added a comment to "vmbus_add_channel_kobj()" to describe how the
empty directory is removed if "sysfs_create_group()" returns an
error.
- Updated the commit message.
NOTE: “.default_attrs” must be removed from vmbus_chan_ktype in order to
use the is_visible() function because "default_attrs" is an array of
attributes, not an attribute_group.
Changes in v4:
- Added “is_visible()” callback functions for the device-level and
channel-level attribute groups.
- Removed the separate monitor attribute groups proposed in v3. They’re
no longer needed because the “is_visible()” callbacks are used to
determine the attribute visibility.
- Removed "default_attributes" from "vmbus_chan_attrs" and created a
channel-level attribute group.
- Removed the "kobject_put(kobj)" call proposed in v3. The calling
functions take care of calling "kobject_put(channel->kobj)" if an
error is returned by "vmbus_add_channel_kobj()".
- Updated the commit message and subject for clarity and to reflect the
new changes in v4.
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/channel_mgmt.c | 1 +
drivers/hv/hyperv_vmbus.h | 2 +
drivers/hv/vmbus_drv.c | 77 +++++++++++++++++++++++-
4 files changed, 87 insertions(+), 5 deletions(-)
diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus
index 826689dcc2e6..8e8d167eca31 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 <[email protected]>
-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 <[email protected]>
-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 <[email protected]>
-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/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index f174b38f390f..3fc0b247a807 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -347,6 +347,7 @@ static struct vmbus_channel *alloc_channel(void)
static void free_channel(struct vmbus_channel *channel)
{
tasklet_kill(&channel->callback_event);
+ vmbus_remove_channel_attr_group(channel);
kobject_put(&channel->kobj);
}
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 50fabf4dcb75..e5467b821f41 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -322,6 +322,8 @@ void vmbus_device_unregister(struct hv_device *device_obj);
int vmbus_add_channel_kobj(struct hv_device *device_obj,
struct vmbus_channel *channel);
+void vmbus_remove_channel_attr_group(struct vmbus_channel *channel);
+
struct vmbus_channel *relid2channel(u32 relid);
void vmbus_free_channels(void);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 41f782af73ea..aa25f3bcbdea 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -630,7 +630,36 @@ static struct attribute *vmbus_dev_attrs[] = {
&dev_attr_driver_override.attr,
NULL,
};
-ATTRIBUTE_GROUPS(vmbus_dev);
+
+/*
+ * Device-level attribute_group callback function. Returns the permission for
+ * each attribute, and returns 0 if an attribute is not visible.
+ */
+static umode_t vmbus_dev_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ const struct hv_device *hv_dev = device_to_hv_device(dev);
+
+ /* Hide the monitor attributes if the monitor mechanism is not used. */
+ if (!hv_dev->channel->offermsg.monitor_allocated &&
+ (attr == &dev_attr_monitor_id.attr ||
+ attr == &dev_attr_server_monitor_pending.attr ||
+ attr == &dev_attr_client_monitor_pending.attr ||
+ attr == &dev_attr_server_monitor_latency.attr ||
+ attr == &dev_attr_client_monitor_latency.attr ||
+ attr == &dev_attr_server_monitor_conn_id.attr ||
+ attr == &dev_attr_client_monitor_conn_id.attr))
+ return 0;
+
+ return attr->mode;
+}
+
+static const struct attribute_group vmbus_dev_group = {
+ .attrs = vmbus_dev_attrs,
+ .is_visible = vmbus_dev_attr_is_visible
+};
+__ATTRIBUTE_GROUPS(vmbus_dev);
/*
* vmbus_uevent - add uevent for our device
@@ -1583,10 +1612,34 @@ static struct attribute *vmbus_chan_attrs[] = {
NULL
};
+/*
+ * Channel-level attribute_group callback function. Returns the permission for
+ * each attribute, and returns 0 if an attribute is not visible.
+ */
+static umode_t vmbus_chan_attr_is_visible(struct kobject *kobj,
+ struct attribute *attr, int idx)
+{
+ const struct vmbus_channel *channel =
+ container_of(kobj, struct vmbus_channel, kobj);
+
+ /* Hide the monitor attributes if the monitor mechanism is not used. */
+ if (!channel->offermsg.monitor_allocated &&
+ (attr == &chan_attr_pending.attr ||
+ attr == &chan_attr_latency.attr ||
+ attr == &chan_attr_monitor_id.attr))
+ return 0;
+
+ return attr->mode;
+}
+
+static struct attribute_group vmbus_chan_group = {
+ .attrs = vmbus_chan_attrs,
+ .is_visible = vmbus_chan_attr_is_visible
+};
+
static struct kobj_type vmbus_chan_ktype = {
.sysfs_ops = &vmbus_chan_sysfs_ops,
.release = vmbus_chan_release,
- .default_attrs = vmbus_chan_attrs,
};
/*
@@ -1594,6 +1647,7 @@ static struct kobj_type vmbus_chan_ktype = {
*/
int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
{
+ const struct device *device = &dev->device;
struct kobject *kobj = &channel->kobj;
u32 relid = channel->offermsg.child_relid;
int ret;
@@ -1604,11 +1658,30 @@ int vmbus_add_channel_kobj(struct hv_device *dev, struct vmbus_channel *channel)
if (ret)
return ret;
+ ret = sysfs_create_group(kobj, &vmbus_chan_group);
+
+ if (ret) {
+ /*
+ * The calling functions' error handling paths will cleanup the
+ * empty channel directory.
+ */
+ dev_err(device, "Unable to set up channel sysfs files\n");
+ return ret;
+ }
+
kobject_uevent(kobj, KOBJ_ADD);
return 0;
}
+/*
+ * vmbus_remove_channel_attr_group - remove the channel's attribute group
+ */
+void vmbus_remove_channel_attr_group(struct vmbus_channel *channel)
+{
+ sysfs_remove_group(&channel->kobj, &vmbus_chan_group);
+}
+
/*
* vmbus_device_create - Creates and registers a new child device
* on the vmbus.
--
2.17.1
On Tue, Mar 19, 2019 at 12:04:01AM -0400, Kimberly Brown wrote:
> 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.
>
> Add "is_visible()" callback functions for the device-level and
> channel-level attribute groups. These functions will hide the monitor
> sysfs files when the monitor mechanism is not used.
>
> Remove ".default_attributes" from "vmbus_chan_attrs" and create a
> channel-level attribute group. These changes allow the new
> "is_visible()" callback function to be applied to the channel-level
> attributes.
>
> Call "sysfs_create_group()" in "vmbus_add_channel_kobj()" to create the
> channel's sysfs files. Add a new function,
> “vmbus_remove_channel_attr_group()”, and call it in "free_channel()" to
> remove the channel's sysfs files when the channel is closed.
>
> Signed-off-by: Kimberly Brown <[email protected]>
Nice work, thanks for all of the changes you have made to this over
time.
Reviewed-by: Greg Kroah-Hartman <[email protected]>
From: Kimberly Brown <[email protected]> Sent: Monday, March 18, 2019 9:04 PM
>
> 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.
>
> Add "is_visible()" callback functions for the device-level and
> channel-level attribute groups. These functions will hide the monitor
> sysfs files when the monitor mechanism is not used.
>
> Remove ".default_attributes" from "vmbus_chan_attrs" and create a
> channel-level attribute group. These changes allow the new
> "is_visible()" callback function to be applied to the channel-level
> attributes.
>
> Call "sysfs_create_group()" in "vmbus_add_channel_kobj()" to create the
> channel's sysfs files. Add a new function,
> “vmbus_remove_channel_attr_group()”, and call it in "free_channel()" to
> remove the channel's sysfs files when the channel is closed.
>
> Signed-off-by: Kimberly Brown <[email protected]>
Reviewed-by: Michael Kelley <[email protected]>
On Tue, Mar 19, 2019 at 12:04:01AM -0400, Kimberly Brown wrote:
>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.
>
>Add "is_visible()" callback functions for the device-level and
>channel-level attribute groups. These functions will hide the monitor
>sysfs files when the monitor mechanism is not used.
>
>Remove ".default_attributes" from "vmbus_chan_attrs" and create a
>channel-level attribute group. These changes allow the new
>"is_visible()" callback function to be applied to the channel-level
>attributes.
>
>Call "sysfs_create_group()" in "vmbus_add_channel_kobj()" to create the
>channel's sysfs files. Add a new function,
>“vmbus_remove_channel_attr_group()”, and call it in "free_channel()" to
>remove the channel's sysfs files when the channel is closed.
>
>Signed-off-by: Kimberly Brown <[email protected]>
I've queued it up for hyperv-fixes, thanks Kimberly.
Should this go in stable as well?
--
Thanks,
Sasha
From: Sasha Levin <[email protected]> Sent: Wednesday, March 20, 2019 8:57 PM
>
> I've queued it up for hyperv-fixes, thanks Kimberly.
>
> Should this go in stable as well?
>
My take: these changes lean more toward being a "clean up" rather
than fixing a problem that has significant impact. The data in the sysfs
entries is bogus, but isn't actually hurting anything. The current code does
index off the end of an array in some cases, but the accesses are reads
and there's plenty of padding so that actually making an invalid memory
reference won't happen. In the interest of reducing churn in the
stable branches, I would lean toward *not* backporting this to stable.
Michael