| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190226081848.GA15659@kroah.com>
Date: Tue, 26 Feb 2019 09:18:48 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Kimberly Brown <kimbrownkd@...il.com>
Cc: Michael Kelley <mikelley@...rosoft.com>,
Long Li <longli@...rosoft.com>,
Sasha Levin <Alexander.Levin@...rosoft.com>,
Stephen Hemminger <sthemmin@...rosoft.com>,
Dexuan Cui <decui@...rosoft.com>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
Haiyang Zhang <haiyangz@...rosoft.com>,
linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] Drivers: hv: vmbus: Expose monitor data when channel
uses monitor pages
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 <kimbrownkd@...il.com>
> ---
> 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@...rosoft.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@...rosoft.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@...rosoft.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
Powered by blists - more mailing lists