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 <kimbrow...@gmail.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 <sthem...@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 <sthem...@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 <sthem...@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