RE: [PATCH v6] Drivers: hv: vmbus: Expose monitor data only when monitor pages are used
From: Sasha Levin 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
Re: [PATCH v6] Drivers: hv: vmbus: Expose monitor data only when monitor pages are used
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 I've queued it up for hyperv-fixes, thanks Kimberly. Should this go in stable as well? -- Thanks, Sasha
RE: [PATCH v6] Drivers: hv: vmbus: Expose monitor data only when monitor pages are used
From: Kimberly Brown 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 Reviewed-by: Michael Kelley
Re: [PATCH v6] Drivers: hv: vmbus: Expose monitor data only when monitor pages are used
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 Nice work, thanks for all of the changes you have made to this over time. Reviewed-by: Greg Kroah-Hartman
