[PATCH v2 1/2] Drivers: hv: vmbus: Change server monitor_pages index to 0
Change the monitor_pages index in server_monitor_pending_show() to '0'. '0' is the correct monitor_pages index for the server. A comment for the monitor_pages field in the vmbus_connection struct definition indicates that the 1st page is for parent->child notifications. In addition, the server_monitor_latency_show() and server_monitor_conn_id_show() functions use monitor_pages index '0'. Signed-off-by: Kimberly Brown Acked-by: Stephen Hemminger --- drivers/hv/vmbus_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 403fee01572c..f2a79f5129d7 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -234,7 +234,7 @@ static ssize_t server_monitor_pending_show(struct device *dev, return -ENODEV; return sprintf(buf, "%d\n", channel_pending(hv_dev->channel, - vmbus_connection.monitor_pages[1])); + vmbus_connection.monitor_pages[0])); } static DEVICE_ATTR_RO(server_monitor_pending); -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] Drivers: hv: vmbus: Return -EINVAL if monitor_allocated not set
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 --- 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//channels//latency Date: September. 2017 KernelVersion: 4.14 Contact: Stephen Hemminger -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//channels//out_mask @@ -95,7 +98,10 @@ What: /sys/bus/vmbus/devices//channels//pending Date: September. 2017 KernelVersion: 4.14 Contact: Stephen Hemminger -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//channels//read_avail @@ -137,7 +143,10 @@ What: /sys/bus/vmbus/devices//channels//monitor_id Date: January. 2018 KernelVersion: 4.16 Contact: Stephen Hemminger -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//channels//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-&g
[PATCH v2 0/2] Drivers: hv: vmbus: Fix sysfs functions that display monitor id and page data
Some of the monitor id and monitor page data sysfs files display incorrect data. This patchset provides the following changes: 1) Change the monitor_pages index in server_monitor_pending_show() to '0', which is the correct index for the server. 2) If monitor pages are not allocated to a channel, return -EINVAL when the channel's monitor id and monitor page data sysfs files are opened. The data that is currently shown in sysfs is incorrect. Changes in v2: Patch 1: "Change server monitor_pages index to 0" - Added the "Acked-by" line received for v1. Patch 2: "Return -EINVAL if monitor_allocated not set" - Changed the return value for cases where monitor_allocated is not set to "-EINVAL" as suggested by S. Hemminger. - Updated the commit message to provide more details about the monitor page mechanism as suggested by S. Hemminger. - Updated the sysfs documentation to describe the new return value. Kimberly Brown (2): Drivers: hv: vmbus: Change server monitor_pages index to 0 Drivers: hv: vmbus: Return -EINVAL if monitor_allocated not set Documentation/ABI/stable/sysfs-bus-vmbus | 15 +++-- drivers/hv/vmbus_drv.c | 39 +++- 2 files changed, 50 insertions(+), 4 deletions(-) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
On Thu, Feb 14, 2019 at 08:54:31PM -0500, Sasha Levin wrote: > On Sat, Feb 02, 2019 at 03:07:35PM -0500, Kimberly Brown wrote: > > On Fri, Feb 01, 2019 at 06:24:24PM +, Dexuan Cui wrote: > > > > From: Kimberly Brown > > > > Sent: Thursday, January 31, 2019 9:47 AM > > > > ... > > > > 2) Prevent a deadlock that can occur between the proposed mutex_lock() > > > > call in the vmbus_chan_attr_show() function and the sysfs/kernfs > > > > functions. > > > Hi Kim, > > > Can you please share more details about the deadlock? > > > It's unclear to me how the deadlock happens. > > > > > > > Hi Dexuan, > > > > I encountered the deadlock by: > > 1) Adding a call to msleep() before acquiring the mutex in > > vmbus_chan_attr_show() > > 2) Opening a hv_netvsc subchannel's sysfs file > > 3) Removing hv_netvsc while the sysfs file is opening > > Dexuan, any objections to pulling this fix in? > Hi Sasha, This fix can cause a deadlock. I'm working on a different fix for the original race condition problem. Thanks, Kim > -- > Thanks, > Sasha ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Drivers: hv: vmbus: Display nothing in sysfs if monitor_allocated not set
On Mon, Feb 11, 2019 at 10:02:47AM -0800, Stephen Hemminger wrote:
> On Mon, 11 Feb 2019 02:01:18 -0500
> Kimberly Brown wrote:
>
> > On Fri, Feb 08, 2019 at 02:32:09PM -0800, Stephen Hemminger wrote:
> > > On Fri, 8 Feb 2019 05:01:12 -0500
> > > Kimberly Brown wrote:
> > >
> > > You are right, the current behavior is broken.
> > > It would be good to add a description of under what conditions
> > > monitor is not used. Is this some part of a project emulating
> > > Hyper-V?
> > >
> >
> > I'm not sure which conditions determine whether the monitor mechanism is
> > used. I've searched the Hypervisor TLFS, and I couldn't find any
> > information. If you have any suggestions for where I can find this
> > information, please let me know.
>
> The monitor page stuff pre-dates my involvement with Hyper-V. KY might know.
> But based on comments it looks like it was added to avoid hypercalls
> for each message. It probably showed up in Windows Server 2012 timeframe.
>
> To test you might want to dig up Windows Server 2008.
>
It looks like the monitor mechanism has always been used. It's present in the
earliest commit that I can find: 3e7ee4902fe6 ("add the Hyper-V virtual bus")
from 2009.
I propose that the following sentences be added to the sysfs documentation for
the affected attributes:
"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."
I think that this provides sufficient information for a user to understand why
opening an affected file can return EINVAL. What do you think?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Drivers: hv: vmbus: Display nothing in sysfs if monitor_allocated not set
On Fri, Feb 08, 2019 at 02:32:09PM -0800, Stephen Hemminger wrote: > On Fri, 8 Feb 2019 05:01:12 -0500 > Kimberly Brown wrote: > > You are right, the current behavior is broken. > It would be good to add a description of under what conditions > monitor is not used. Is this some part of a project emulating > Hyper-V? > I'm not sure which conditions determine whether the monitor mechanism is used. I've searched the Hypervisor TLFS, and I couldn't find any information. If you have any suggestions for where I can find this information, please let me know. No, I'm not working on a project emulating Hyper-V. > > > + > > + if (!hv_dev->channel->offermsg.monitor_allocated) > > + return sprintf(buf, "\n"); > > If monitor is not used, why not return an error instead of empty > data. Any program (or user) would have to handle that already. I think that returning an error instead is fine. I'll make this change in the next version of the patch. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] Drivers: hv: vmbus: Display nothing in sysfs if monitor_allocated not set
If monitor pages are not allocated to a channel, the channel does not have a valid monitor id or valid monitor page data. In these cases, some of the "_show" functions display incorrect data. The "_show" functions that display monitor page data access and display data that is beyond the bounds of the hv_monitor_page array fields, which is obviously incorrect. The "_show" functions that display the monitor id display an invalid monitor id. 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, display nothing. Signed-off-by: Kimberly Brown --- Documentation/ABI/stable/sysfs-bus-vmbus | 9 -- drivers/hv/vmbus_drv.c | 37 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus index 3fed8fdb873d..af52be4ffc5d 100644 --- a/Documentation/ABI/stable/sysfs-bus-vmbus +++ b/Documentation/ABI/stable/sysfs-bus-vmbus @@ -81,7 +81,8 @@ What: /sys/bus/vmbus/devices//channels//latency Date: September. 2017 KernelVersion: 4.14 Contact: Stephen Hemminger -Description: Channel signaling latency +Description: Channel signaling latency. If monitor pages are not allocated + to the channel, nothing is displayed. Users: Debugging tools What: /sys/bus/vmbus/devices//channels//out_mask @@ -95,7 +96,8 @@ What: /sys/bus/vmbus/devices//channels//pending Date: September. 2017 KernelVersion: 4.14 Contact: Stephen Hemminger -Description: Channel interrupt pending state +Description: Channel interrupt pending state. If monitor pages are not + allocated to the channel, nothing is displayed. Users: Debugging tools What: /sys/bus/vmbus/devices//channels//read_avail @@ -137,7 +139,8 @@ What: /sys/bus/vmbus/devices//channels//monitor_id Date: January. 2018 KernelVersion: 4.16 Contact: Stephen Hemminger -Description: Monitor bit associated with channel +Description: Monitor bit associated with channel. If monitor pages are not + allocated to the channel, nothing is displayed. Users: Debugging tools and userspace drivers What: /sys/bus/vmbus/devices//channels//ring diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index f2a79f5129d7..c88a3623be56 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 sprintf(buf, "\n"); + 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 sprintf(buf, "\n"); + 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 sprintf(buf, "\n"); + 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 sprintf(buf, "\n"); + 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 sprintf(buf, "\n"); + return sprintf(buf, "%d\n", channel_latency(hv_dev->channel, vm
[PATCH 1/2] Drivers: hv: vmbus: Change server monitor_pages index to 0
Change the monitor_pages index in server_monitor_pending_show() to '0'. '0' is the correct monitor_pages index for the server. A comment for the monitor_pages field in the vmbus_connection struct definition indicates that the 1st page is for parent->child notifications. In addition, the server_monitor_latency_show() and server_monitor_conn_id_show() functions use monitor_pages index '0'. Signed-off-by: Kimberly Brown --- drivers/hv/vmbus_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 403fee01572c..f2a79f5129d7 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -234,7 +234,7 @@ static ssize_t server_monitor_pending_show(struct device *dev, return -ENODEV; return sprintf(buf, "%d\n", channel_pending(hv_dev->channel, - vmbus_connection.monitor_pages[1])); + vmbus_connection.monitor_pages[0])); } static DEVICE_ATTR_RO(server_monitor_pending); -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/2] Drivers: hv: vmbus: Fix sysfs functions that display monitor id and page data
Some of the monitor id and monitor page data sysfs files display incorrect data. This patchset provides the following changes: 1) Change the monitor_pages index in server_monitor_pending_show() to '0', which is the correct index for the server. 2) If monitor pages are not allocated to a channel, display nothing in the channel's monitor id and monitor page data sysfs files. The data that is currently shown in sysfs is incorrect. Kimberly Brown (2): Drivers: hv: vmbus: Change server monitor_pages index to 0 Drivers: hv: vmbus: Display nothing in sysfs if monitor_allocated not set Documentation/ABI/stable/sysfs-bus-vmbus | 9 -- drivers/hv/vmbus_drv.c | 39 +++- 2 files changed, 44 insertions(+), 4 deletions(-) -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4] Drivers: hv: vmbus: Expose counters for interrupts and full conditions
Counter values for per-channel interrupts and ring buffer full
conditions are useful for investigating performance.
Expose counters in sysfs for 2 types of guest to host interrupts:
1) Interrupts caused by the channel's outbound ring buffer transitioning
from empty to not empty
2) Interrupts caused by the channel's inbound ring buffer transitioning
from full to not full while a packet is waiting for enough buffer space to
become available
Expose 2 counters in sysfs for the number of times that write operations
encountered a full outbound ring buffer:
1) The total number of write operations that encountered a full
condition
2) The number of write operations that were the first to encounter a
full condition
Increment the outbound full condition counters in the
hv_ringbuffer_write() function because, for most drivers, a full
outbound ring buffer is detected in that function. Also increment the
outbound full condition counters in the set_channel_pending_send_size()
function. In the hv_sock driver, a full outbound ring buffer is detected
and set_channel_pending_send_size() is called before
hv_ringbuffer_write() is called.
I tested this patch by confirming that the sysfs files were created and
observing the counter values. The values seemed to increase by a
reasonable amount when the Hyper-v related drivers were in use.
Signed-off-by: Kimberly Brown
---
Changes in v4:
- In the commit message, added a paragraph describing why the full
condition counters are incremented in two functions.
- In the four new "_show" functions, added a cast to "(unsigned long
long)" in the sprintf() calls. This problem was reported by S.
Hemminger.
- In the vmbus_channel struct definition, moved the three new fields
pertaining to full ring buffers ("intr_in_full", "out_full_total",
"out_full_first") to the bottom of the struct. These fields should
not be used often because full ring buffers should not be encountered
often. This change addresses S. Hemminger's concern about the new
fields causing additional cache misses in the hot path.
- In the set_channel_pending_send_size() function, moved the
acquire/release of the spinlock to inside the if-statement. The
spinlock needs to protect only the incrementing variables. Since full
ring buffers should not be encountered often, the addition of this
spinlock acquire/release should not affect performance. This change
addresses S. Hemminger's concern that additional locking will affect
performance.
NOTE: S. Hemminger also requested that I consider placing the new bool
field, "out_full_flag", in a bitfield. I chose not to make this
change because setting an individual bit in a bitfield is less
efficient than setting the value of a bool.
Changes in v3:
- Used the outbound ring buffer spinlock to protect the the full
condition counters in set_channel_pending_send_size()
- Corrected the KernelVersion values for the new entries in
Documentation/ABI/stable/sysfs-bus-vmbus
Changes in v2:
- Added mailing lists to the cc list
- Removed the host to guest interrupt counters proposed in v1 because
they were not accurate
- Added full condition counters for the channel's outbound ring buffer
Documentation/ABI/stable/sysfs-bus-vmbus | 33 +
drivers/hv/ring_buffer.c | 14 +++-
drivers/hv/vmbus_drv.c | 36 +++
include/linux/hyperv.h | 46
4 files changed, 128 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus
b/Documentation/ABI/stable/sysfs-bus-vmbus
index 3fed8fdb873d..826689dcc2e6 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -146,3 +146,36 @@ KernelVersion: 4.16
Contact: Stephen Hemminger
Description: Binary file created by uio_hv_generic for ring buffer
Users: Userspace drivers
+
+What: /sys/bus/vmbus/devices//channels//intr_in_full
+Date: February 2019
+KernelVersion: 5.0
+Contact:Michael Kelley
+Description:Number of guest to host interrupts caused by the inbound ring
+ buffer transitioning from full to not full while a packet is
+ waiting for buffer space to become available
+Users: Debugging tools
+
+What: /sys/bus/vmbus/devices//channels//intr_out_empty
+Date: February 2019
+KernelVersion: 5.0
+Contact:Michael Kelley
+Description:Number of guest to host interrupts caused by the outbound ring
+ buffer transitioning from empty to not empty
+Users: Debugging tools
+
+What: /sys/bus/vmbus/devices//channels//out_full_first
+Date: February 2019
+KernelVersion: 5.0
+Contact:Michael Kelley
+Description:Number of write operations that were the first to encounter a
Re: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
On Fri, Feb 01, 2019 at 06:24:24PM +, Dexuan Cui wrote:
> > From: Kimberly Brown
> > Sent: Thursday, January 31, 2019 9:47 AM
> > ...
> > 2) Prevent a deadlock that can occur between the proposed mutex_lock()
> > call in the vmbus_chan_attr_show() function and the sysfs/kernfs functions.
> Hi Kim,
> Can you please share more details about the deadlock?
> It's unclear to me how the deadlock happens.
>
Hi Dexuan,
I encountered the deadlock by:
1) Adding a call to msleep() before acquiring the mutex in
vmbus_chan_attr_show()
2) Opening a hv_netvsc subchannel's sysfs file
3) Removing hv_netvsc while the sysfs file is opening
Here's the lockdep output:
[ 164.167699] hv_vmbus: unregistering driver hv_netvsc
[ 164.171660] ==
[ 164.171661] WARNING: possible circular locking dependency detected
[ 164.171663] 5.0.0-rc1+ #58 Not tainted
[ 164.171664] --
[ 164.171666] kworker/0:1/12 is trying to acquire lock:
[ 164.171668] 664f9893 (kn->count#43){}, at:
kernfs_remove+0x23/0x40
[ 164.171676]
but task is already holding lock:
[ 164.171677] 7b9e8443 (_connection.channel_mutex){+.+.}, at:
vmbus_onoffer_rescind+0x1ae/0x210 [hv_vmbus]
[ 164.171686]
which lock already depends on the new lock.
[ 164.171687]
the existing dependency chain (in reverse order) is:
[ 164.171688]
-> #1 (_connection.channel_mutex){+.+.}:
[ 164.171694]__mutex_lock+0x65/0x9b0
[ 164.171696]mutex_lock_nested+0x1b/0x20
[ 164.171700]vmbus_chan_attr_show+0x3f/0x90 [hv_vmbus]
[ 164.171703]sysfs_kf_seq_show+0xa4/0x130
[ 164.171705]kernfs_seq_show+0x2d/0x30
[ 164.171708]seq_read+0xe2/0x410
[ 164.171711]kernfs_fop_read+0x14e/0x1a0
[ 164.171714]__vfs_read+0x3a/0x1a0
[ 164.171716]vfs_read+0x91/0x140
[ 164.171718]ksys_read+0x58/0xc0
[ 164.171721]__x64_sys_read+0x1a/0x20
[ 164.171724]do_syscall_64+0x65/0x1b0
[ 164.171727]entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 164.171728]
-> #0 (kn->count#43){}:
[ 164.171732]lock_acquire+0xa3/0x180
[ 164.171734]__kernfs_remove+0x278/0x300
[ 164.171737]kernfs_remove+0x23/0x40
[ 164.171739]sysfs_remove_dir+0x51/0x60
[ 164.171741]kobject_del.part.7+0x13/0x40
[ 164.171743]kobject_put+0x6a/0x1a0
[ 164.171748]hv_process_channel_removal+0xfe/0x180 [hv_vmbus]
[ 164.171752]vmbus_onoffer_rescind+0x20a/0x210 [hv_vmbus]
[ 164.171756]vmbus_onmessage+0x5f/0x150 [hv_vmbus]
[ 164.171760]vmbus_onmessage_work+0x22/0x30 [hv_vmbus]
[ 164.171763]process_one_work+0x291/0x5c0
[ 164.171765]worker_thread+0x34/0x400
[ 164.171767]kthread+0x124/0x140
[ 164.171770]ret_from_fork+0x3a/0x50
[ 164.171771]
other info that might help us debug this:
[ 164.171772] Possible unsafe locking scenario:
[ 164.171773]CPU0CPU1
[ 164.171775]
[ 164.171776] lock(_connection.channel_mutex);
[ 164.171777]lock(kn->count#43);
[ 164.171779]
lock(_connection.channel_mutex);
[ 164.171781] lock(kn->count#43);
[ 164.171783]
*** DEADLOCK ***
[ 164.171785] 3 locks held by kworker/0:1/12:
[ 164.171786] #0: 2128b29f ((wq_completion)"events"){+.+.}, at:
process_one_work+0x20f/0x5c0
[ 164.171790] #1: 41d2602c ((work_completion)(>work)){+.+.}, at:
process_one_work+0x20f/0x5c0
[ 164.171794] #2: 7b9e8443 (_connection.channel_mutex){+.+.},
at: vmbus_onoffer_rescind+0x1ae/0x210 [hv_vmbus]
[ 164.171799]
stack backtrace:
[ 164.171802] CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.0.0-rc1+ #58
[ 164.171804] Hardware name: Microsoft Corporation Virtual Machine/Virtual
Machine, BIOS Hyper-V UEFI Release v3.0 03/02/2018
[ 164.171809] Workqueue: events vmbus_onmessage_work [hv_vmbus]
[ 164.171811] Call Trace:
[ 164.171816] dump_stack+0x8e/0xd5
[ 164.171819] print_circular_bug.isra.37+0x1e7/0x1f5
[ 164.171822] __lock_acquire+0x1427/0x1490
[ 164.171826] ? sched_clock+0x9/0x10
[ 164.171830] lock_acquire+0xa3/0x180
[ 164.171832] ? lock_acquire+0xa3/0x180
[ 164.171835] ? kernfs_remove+0x23/0x40
[ 164.171838] __kernfs_remove+0x278/0x300
[ 164.171840] ? kernfs_remove+0x23/0x40
[ 164.171843] kernfs_remove+0x23/0x40
[ 164.171846] sysfs_remove_dir+0x51/0x60
[ 164.171848] kobject_del.part.7+0x13/0x40
[ 164.171850] kobject_put+0x6a/0x1a0
[ 164.171855] hv_process_channel_removal+0xfe/0x180 [hv_vmbus]
[ 164.171859] vmbus_onoffer_rescind+0x20a/0x210 [hv_vmbus]
[ 164.171863] vmbus_onmessage+0x5f/0x150 [hv_vmbus
Re: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
On Thu, Jan 31, 2019 at 04:45:35PM +, Michael Kelley wrote: > From: Sasha Levin Sent: Thursday, January 31, 2019 7:20 AM > > > > I've queued this one for hyper-fixes, thanks all! > > > > Actually, please hold off on queuing this one. In a conversation I had > yesterday with Kim, they had identified a deadlock. Kim was going to be > looking at some revisions to avoid the deadlock. > > Kim -- please confirm. > > Michael This is correct. I need to send a v2 of this patch which addresses two issues: 1) Check channel->state inside the mutex acquire/release in vmbus_chan_attr_show(). 2) Prevent a deadlock that can occur between the proposed mutex_lock() call in the vmbus_chan_attr_show() function and the sysfs/kernfs functions. I've identified two possible solutions to the deadlock: 1) Add a new mutex to the vmbus_channel struct which protects changes to "channel->state". Use this new mutex in vmbus_chan_attr_show() instead of "vmbus_connection.channel_mutex". 2) Use "vmbus_connection.channel_mutex" in vmbus_chan_attr_show() as originally proposed, and acquire it with mutex_trylock(). If the mutex cannot be acquired, return -EINVAL. I'm leaning towards (2), using mutex_trylock(). "vmbus_connection.channel_mutex" appears to be used only when a channel is being opened or closed, so vmbus_chan_attr_show() should be able to acquire the mutex when the channel is in use. If anyone has suggestions, please let me know. Thanks, Kim ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
On Tue, Jan 22, 2019 at 06:40:30PM +, Dexuan Cui wrote:
> > From: Kimberly Brown
> > Sent: Monday, January 21, 2019 10:43 PM
> > > > @@ -1421,7 +1422,10 @@ static ssize_t vmbus_chan_attr_show(struct
> > > > kobject *kobj,
> > > > if (chan->state != CHANNEL_OPENED_STATE)
> > > > return -EINVAL;
> > > >
> > > > - return attribute->show(chan, buf);
> > > > + mutex_lock(_connection.channel_mutex);
> > > > + ret = attribute->show(chan, buf);
> > > > + mutex_unlock(_connection.channel_mutex);
> > > > + return ret;
> > > > }
> > >
> > > It looks this patch is already able to fix the original issue:
> > > 6712cc9c2211 ("vmbus: don't return values for uninitalized channels"),
> > > as chan->state can't be CHANNEL_OPENED_STATE when
> > > channel->ringbuffer_page is not set up yet, or has been freed.
> > > -- Dexuan
> >
> > I think that patch 6712cc9c2211 fixes the problem when the channel is
> > not set up yet, but it doesn't fix the problem when the channel is being
> > closed
> Yeah, now I see your point.
>
> > The channel could be freed after the check that "chan->state" is
> > CHANNEL_OPENED_STATE, while the "attribute->show()" function is running.
>
> IMO the channel struct itself can't be freed while the "attribute->show()"
> function is running, because I suppose the sysfs interface should have an
> extra
> reference to channel->kobj (see vmbus_add_channel_kobj()), so before the sysfs
> files are removed, the channel struct itself can't disappear.
> (I didn't check the related code very carefully, so I could be wrong. :-) )
>
I think that you're correct that the channel struct can't be freed while
the "attribute->show()" function is running. I tested this by calling
msleep() in chan_attr_show(), opening a subchannel sysfs file, and
unloading hv_netvsc. Unloading hv_netvsc should result in calls to
vmbus_chan_release() for each subchannel. I confirmed that
vmbus_chan_release() isn't called until the "attribute->show()" function
returns.
> But as you pointed, at least for sub-channels, channel->ringbuffer_page
> can indeed disappear in vmbus_close() -> ... -> vmbus_free_ring(), and
> the "attribute->show()" could crash when the race happens.
> Adding channel_mutex here seems to be able to fix the race for
> sub-channels, as the same channel_mutex is used in vmbus_disconnect_ring().
>
> For a primary channel, vmbus_close() -> vmbus_free_ring() can still
> free channel->ringbuffer_page without notifying the "attribute->show()".
> We may also need to acquire/release the channel_mutex in vmbus_close()?
>
> > Actually, there should be checks that "chan" is not null and that
> > "chan->state" is CHANNEL_OPENED_STATE within the locked section. I'll
> > need to fix that.
> I suppose "chan" can not be NULL here (see the above).
>
> Checking "chan->state" may not help to completely fix the race, because
> there is no locking/synchronization code in
> vmbus_close_internal() when we test and change "channel->state".
>
The calls to vmbus_close_internal() for the subchannels and the primary
channel are protected with channel_mutex in vmbus_disconnect_ring().
This prevents "channel->state" from changing while "attribute->show()" is
running.
> I guess we may need to check if channel->ringbuffer_page is NULL in
> the "attribute->show()".
>
For the primary channel, vmbus_free_ring() is called after the
return from vmbus_disconnect_ring(). Therefore, the primary channel's
state is changed before "channel->ringbuffer_page" is set to NULL.
Checking the channel state should be sufficient to prevent the ring
buffers from being freed while "attribute->show()" is running. The
ring buffers can't be freed until the channel's state is changed, and
the channel state change is protected by the mutex.
I think checking that "channel->ringbuffer_page" is not NULL would
also work, but, as you stated, we would need to aquire/release
channel_mutex in vmbus_close().
> PS, to prove that a race condition does exist and can really cause a panic or
> something, I usually add some msleep() delays in different paths so that I
> can reproduce the crash every time I take a special action, e.g. when I read
> the sysfs files of a NIC, I try to remove hv_netvsc driver. This way, I can
> prove
> a patch can indeed help, at least it can fix the crash which would happen
> without the patch. :-)
>
Thanks! I was able to free the ring buffers while "attribute->show()"
was running, which caused a null pointer dereference bug. As expected,
the mutex lock fixed it.
> -- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
On Tue, Jan 22, 2019 at 03:46:48AM +, Dexuan Cui wrote:
> > From: Kimberly Brown
> > Sent: Monday, January 21, 2019 6:08 PM
> > Subject: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show
> > functions
> >
> > The channel level "_show" functions are vulnerable to race conditions.
> > Add a mutex lock and unlock around the call to the channel level "_show"
> > functions in vmbus_chan_attr_show().
> >
> > This problem was discussed here:
> > https://lkml.org/lkml/2018/10/18/830
> >
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -1414,6 +1414,7 @@ static ssize_t vmbus_chan_attr_show(struct kobject
> > *kobj,
> > = container_of(attr, struct vmbus_chan_attribute, attr);
> > const struct vmbus_channel *chan
> > = container_of(kobj, struct vmbus_channel, kobj);
> > + ssize_t ret;
> >
> > if (!attribute->show)
> > return -EIO;
> > @@ -1421,7 +1422,10 @@ static ssize_t vmbus_chan_attr_show(struct
> > kobject *kobj,
> > if (chan->state != CHANNEL_OPENED_STATE)
> > return -EINVAL;
> >
> > - return attribute->show(chan, buf);
> > + mutex_lock(_connection.channel_mutex);
> > + ret = attribute->show(chan, buf);
> > + mutex_unlock(_connection.channel_mutex);
> > + return ret;
> > }
>
> It looks this patch is already able to fix the original issue:
> 6712cc9c2211 ("vmbus: don't return values for uninitalized channels"),
> as chan->state can't be CHANNEL_OPENED_STATE when
> channel->ringbuffer_page is not set up yet, or has been freed.
>
> Thanks,
> -- Dexuan
I think that patch 6712cc9c2211 fixes the problem when the channel is
not set up yet, but it doesn't fix the problem when the channel is being
closed. The channel could be freed after the check that "chan->state" is
CHANNEL_OPENED_STATE, while the "attribute->show()" function is running.
Actually, there should be checks that "chan" is not null and that
"chan->state" is CHANNEL_OPENED_STATE within the locked section. I'll
need to fix that.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
The channel level "_show" functions are vulnerable to race conditions.
Add a mutex lock and unlock around the call to the channel level "_show"
functions in vmbus_chan_attr_show().
This problem was discussed here: https://lkml.org/lkml/2018/10/18/830
Signed-off-by: Kimberly Brown
---
drivers/hv/vmbus_drv.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 403fee01572c..e8189bc168da 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1414,6 +1414,7 @@ static ssize_t vmbus_chan_attr_show(struct kobject *kobj,
= container_of(attr, struct vmbus_chan_attribute, attr);
const struct vmbus_channel *chan
= container_of(kobj, struct vmbus_channel, kobj);
+ ssize_t ret;
if (!attribute->show)
return -EIO;
@@ -1421,7 +1422,10 @@ static ssize_t vmbus_chan_attr_show(struct kobject *kobj,
if (chan->state != CHANNEL_OPENED_STATE)
return -EINVAL;
- return attribute->show(chan, buf);
+ mutex_lock(_connection.channel_mutex);
+ ret = attribute->show(chan, buf);
+ mutex_unlock(_connection.channel_mutex);
+ return ret;
}
static const struct sysfs_ops vmbus_chan_sysfs_ops = {
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] Drivers: hv: vmbus: Expose counters for interrupts and full conditions
On Thu, Jan 17, 2019 at 09:11:03AM -0800, Stephen Hemminger wrote:
>
>
> > +static ssize_t channel_intr_in_full_show(const struct vmbus_channel
> > *channel,
> > +char *buf)
> > +{
> > + return sprintf(buf, "%llu\n", channel->intr_in_full);
> > +}
>
>
> intr_in_full is u64, which is not the same as unsigned long long.
> to be correct you need a cast here.
>
Thanks for the feedback. I'll fix this issue in all four of the "_show"
functions that are added in this patch.
> > > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> > > index dcb6977afce9..7e5239123276 100644
> > > --- a/include/linux/hyperv.h
> > > +++ b/include/linux/hyperv.h
> > > @@ -751,6 +751,27 @@ struct vmbus_channel {
> > > u64 interrupts; /* Host to Guest interrupts */
> > > u64 sig_events; /* Guest to Host events */
> > >
> > > + /* Interrupt counts for 2 types of Guest to Host interrupts */
> > > + u64 intr_in_full; /* in ring buffer, full to not full */
> > > + u64 intr_out_empty; /* out ring buffer, empty to not empty */
> > > +
> > > + /*
> > > + * The total number of write operations that encountered a full
> > > + * outbound ring buffer.
> > > + */
> > > + u64 out_full_total;
> > > + /*
> > > + * The number of write operations that were the first to encounter a
> > > + * full outbound ring buffer.
> > > + */
> > > + u64 out_full_first;
>
> Adding more fields changes cache layout which can cause
> additional cache miss in the hot path.
>
Good point. I think that the "intr_out_empty" field is in a good
location, but the "intr_in_full", "out_full_first", and "out_full_total"
fields could be moved to the end of the struct. These variables are used
only when ring buffer full conditions occur. Ring buffer full conditions
shouldn't be encountered often, and, if they are, they're a signal that
changes should probably be made to prevent them.
If you have any other suggestions for this, please let me know.
> > > + /*
> > > + * Indicates that a full outbound ring buffer was encountered. The flag
> > > + * is set to true when a full outbound ring buffer is encountered and
> > > + * set to false when a write to the outbound ring buffer is completed.
> > > + */
> > > + bool out_full_flag;
>
> Discussion on kernel mailing list. Recommends against putting bool
> in structures since that pads to full sizeof(int). Could this be
> part of a bitfield?
>
There are currently 4 other bool variables in this struct. Maybe some or
all of the bool variables could be placed adjacent to each other and
changed into bitfields. I'll need to look into this.
> > > /* Channel callback's invoked in softirq context */
> > > struct tasklet_struct callback_event;
> > > void (*onchannel_callback)(void *context);
> > > @@ -936,6 +957,23 @@ static inline void *get_per_channel_state(struct
> > > vmbus_channel *c)
> > > static inline void set_channel_pending_send_size(struct vmbus_channel *c,
> > >u32 size)
> > > {
> > > + unsigned long flags;
> > > +
> > > + spin_lock_irqsave(>outbound.ring_lock, flags);
> > > +
> > > + if (size) {
> > > + ++c->out_full_total;
> > > +
> > > + if (!c->out_full_flag) {
> > > + ++c->out_full_first;
> > > + c->out_full_flag = true;
> > > + }
> > > + } else {
> > > + c->out_full_flag = false;
> > > + }
> > > +
> > > + spin_unlock_irqrestore(>outbound.ring_lock, flags);
>
> If this is called often, the additional locking will impact performance.
>
In hv_sock, each call of "hvs_stream_has_space()" results in a call to
"channel_set_pending_send_size()", so this could be a concern. I'll work
on addressing this issue.
> > > c->outbound.ring_buffer->pending_send_sz = size;
> > > }
> > >
>
> Could I propose another alternative.
>
> It might be more useful to count the guest to host interaction events
> rather than the ring buffer.
>
> For example the number of calls to:
> vmbus_set_event which means host exit call
> vmbus_setevent fastpath using sync_set_bit
> calls to rinbuffer_write that returned -EAGAIN
>
> These would require less locking, reuse existing code paths
> and not require additional state.
>
I'm not sure that this approach would provide the data that we're
looking for. For example, we're interested in evaluating how often ring
buffer write operations encounter full conditions. For this, we need to
know how many interaction events were caused by the ring buffer being
full. Counting the number of calls to "vmbus_set_event()" and
"vmbus_setevent()" wouldn't allow us to determine what caused the events.
For counting the full conditions, the number of calls to
"ring_buffer_write()" that returned -EAGAIN isn't sufficient because
hv_sock doesn't use the -EAGAIN path to determine that the ring buffer is
full. Therefore, we need to count the number of full conditions in both
[PATCH v3] Drivers: hv: vmbus: Expose counters for interrupts and full conditions
Counter values for per-channel interrupts and ring buffer full
conditions are useful for investigating performance.
Expose counters in sysfs for 2 types of guest to host interrupts:
1) Interrupts caused by the channel's outbound ring buffer transitioning
from empty to not empty
2) Interrupts caused by the channel's inbound ring buffer transitioning
from full to not full while a packet is waiting for enough buffer space to
become available
Expose 2 counters in sysfs for the number of times that write operations
encountered a full outbound ring buffer:
1) The total number of write operations that encountered a full
condition
2) The number of write operations that were the first to encounter a
full condition
I tested this patch by confirming that the sysfs files were created and
observing the counter values. The values seemed to increase by a
reasonable amount when the Hyper-v related drivers were in use.
Signed-off-by: Kimberly Brown
---
Changes in v3:
- Used the outbound ring buffer spinlock to protect the the full
condition counters in set_channel_pending_send_size()
- Corrected the KernelVersion values for the new entries in
Documentation/ABI/stable/sysfs-bus-vmbus
Changes in v2:
- Added mailing lists to the cc list
- Removed the host to guest interrupt counters proposed in v1 because
they were not accurate
- Added full condition counters for the channel's outbound ring buffer
Documentation/ABI/stable/sysfs-bus-vmbus | 33
drivers/hv/ring_buffer.c | 14 -
drivers/hv/vmbus_drv.c | 32
include/linux/hyperv.h | 38
4 files changed, 116 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus
b/Documentation/ABI/stable/sysfs-bus-vmbus
index 3fed8fdb873d..a0304c563467 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -146,3 +146,36 @@ KernelVersion: 4.16
Contact: Stephen Hemminger
Description: Binary file created by uio_hv_generic for ring buffer
Users: Userspace drivers
+
+What: /sys/bus/vmbus/devices//channels//intr_in_full
+Date: January 2019
+KernelVersion: 5.0
+Contact:Michael Kelley
+Description:Number of guest to host interrupts caused by the inbound ring
+ buffer transitioning from full to not full while a packet is
+ waiting for buffer space to become available
+Users: Debugging tools
+
+What: /sys/bus/vmbus/devices//channels//intr_out_empty
+Date: January 2019
+KernelVersion: 5.0
+Contact:Michael Kelley
+Description:Number of guest to host interrupts caused by the outbound ring
+ buffer transitioning from empty to not empty
+Users: Debugging tools
+
+What: /sys/bus/vmbus/devices//channels//out_full_first
+Date: January 2019
+KernelVersion: 5.0
+Contact:Michael Kelley
+Description:Number of write operations that were the first to encounter an
+ outbound ring buffer full condition
+Users: Debugging tools
+
+What: /sys/bus/vmbus/devices//channels//out_full_total
+Date: January 2019
+KernelVersion: 5.0
+Contact:Michael Kelley
+Description:Total number of write operations that encountered an outbound
+ ring buffer full condition
+Users: Debugging tools
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 1f1a55e07733..9e8b31ccc142 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -74,8 +74,10 @@ static void hv_signal_on_write(u32 old_write, struct
vmbus_channel *channel)
* This is the only case we need to signal when the
* ring transitions from being empty to non-empty.
*/
- if (old_write == READ_ONCE(rbi->ring_buffer->read_index))
+ if (old_write == READ_ONCE(rbi->ring_buffer->read_index)) {
+ ++channel->intr_out_empty;
vmbus_setevent(channel);
+ }
}
/* Get the next write location for the specified ring buffer. */
@@ -272,10 +274,19 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
* is empty since the read index == write index.
*/
if (bytes_avail_towrite <= totalbytes_towrite) {
+ ++channel->out_full_total;
+
+ if (!channel->out_full_flag) {
+ ++channel->out_full_first;
+ channel->out_full_flag = true;
+ }
+
spin_unlock_irqrestore(_info->ring_lock, flags);
return -EAGAIN;
}
+ channel->out_full_flag = false;
+
/* Write to the ring buffer */
next_write_location = hv_get_next_write_location(outring_info);
@@ -530,6 +541,7 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
[PATCH v2] Drivers: hv: vmbus: Expose counters for interrupts and full conditions
Counter values for per-channel interrupts and ring buffer full
conditions are useful for investigating performance.
Expose counters in sysfs for 2 types of guest to host interrupts:
1) Interrupts caused by the channel's outbound ring buffer transitioning
from empty to not empty
2) Interrupts caused by the channel's inbound ring buffer transitioning
from full to not full while a packet is waiting for enough buffer space to
become available
Expose 2 counters in sysfs for the number of times that write operations
encountered a full outbound ring buffer:
1) The total number of write operations that encountered a full
condition
2) The number of write operations that were the first to encounter a
full condition
I tested this patch by confirming that the sysfs files were created and
observing the counter values. The values seemed to increase by a
reasonable amount when the Hyper-v related drivers were in use.
Signed-off-by: Kimberly Brown
---
Changes in v2:
- Added mailing lists to the cc list
- Removed the host to guest interrupt counters proposed in v1 because
they were not accurate
- Added full condition counters for the channel's outbound ring buffer
Documentation/ABI/stable/sysfs-bus-vmbus | 33
drivers/hv/ring_buffer.c | 14 +-
drivers/hv/vmbus_drv.c | 32 +++
include/linux/hyperv.h | 32 +++
4 files changed, 110 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus
b/Documentation/ABI/stable/sysfs-bus-vmbus
index 3fed8fdb873d..31dc89989032 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -146,3 +146,36 @@ KernelVersion: 4.16
Contact: Stephen Hemminger
Description: Binary file created by uio_hv_generic for ring buffer
Users: Userspace drivers
+
+What: /sys/bus/vmbus/devices//channels//intr_in_full
+Date: January 2019
+KernelVersion: 4.21
+Contact:Michael Kelley
+Description:Number of guest to host interrupts caused by the inbound ring
+ buffer transitioning from full to not full while a packet is
+ waiting for buffer space to become available
+Users: Debugging tools
+
+What: /sys/bus/vmbus/devices//channels//intr_out_empty
+Date: January 2019
+KernelVersion: 4.21
+Contact:Michael Kelley
+Description:Number of guest to host interrupts caused by the outbound ring
+ buffer transitioning from empty to not empty
+Users: Debugging tools
+
+What: /sys/bus/vmbus/devices//channels//out_full_first
+Date: January 2019
+KernelVersion: 4.21
+Contact:Michael Kelley
+Description:Number of write operations that were the first to encounter an
+ outbound ring buffer full condition
+Users: Debugging tools
+
+What: /sys/bus/vmbus/devices//channels//out_full_total
+Date: January 2019
+KernelVersion: 4.21
+Contact:Michael Kelley
+Description:Total number of write operations that encountered an outbound
+ ring buffer full condition
+Users: Debugging tools
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 64d0c85d5161..be2cbd0bea7d 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -74,8 +74,10 @@ static void hv_signal_on_write(u32 old_write, struct
vmbus_channel *channel)
* This is the only case we need to signal when the
* ring transitions from being empty to non-empty.
*/
- if (old_write == READ_ONCE(rbi->ring_buffer->read_index))
+ if (old_write == READ_ONCE(rbi->ring_buffer->read_index)) {
+ ++channel->intr_out_empty;
vmbus_setevent(channel);
+ }
}
/* Get the next write location for the specified ring buffer. */
@@ -273,10 +275,19 @@ int hv_ringbuffer_write(struct vmbus_channel *channel,
* is empty since the read index == write index.
*/
if (bytes_avail_towrite <= totalbytes_towrite) {
+ ++channel->out_full_total;
+
+ if (!channel->out_full_flag) {
+ ++channel->out_full_first;
+ channel->out_full_flag = true;
+ }
+
spin_unlock_irqrestore(_info->ring_lock, flags);
return -EAGAIN;
}
+ channel->out_full_flag = false;
+
/* Write to the ring buffer */
next_write_location = hv_get_next_write_location(outring_info);
@@ -531,6 +542,7 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
if (curr_write_sz <= pending_sz)
return;
+ ++channel->intr_in_full;
vmbus_setevent(channel);
}
EXPORT_SYMBOL_GPL(hv_pkt_iter_close);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vm
[PATCH v2] staging: mt7621-dma: Add braces around else branches
Add braces around else branches in conditional statements that include
branches with multiple statements. This style complies with the Linux
kernel coding style and improves consistency and readability. Issues found by
checkpatch.
Signed-off-by: Kimberly Brown
Reviewed-by: Matthias Brugger
---
Changes in v2:
- Added people and mailing lists from get_maintainer.pl to the CC list
- Added Reviewed-by line
drivers/staging/mt7621-dma/mtk-hsdma.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/mt7621-dma/mtk-hsdma.c
b/drivers/staging/mt7621-dma/mtk-hsdma.c
index df6ebf41bdea..35556f024aa1 100644
--- a/drivers/staging/mt7621-dma/mtk-hsdma.c
+++ b/drivers/staging/mt7621-dma/mtk-hsdma.c
@@ -418,8 +418,9 @@ static void mtk_hsdma_chan_done(struct mtk_hsdam_engine
*hsdma,
vchan_cookie_complete(>vdesc);
chan_issued = gdma_next_desc(chan);
}
- } else
+ } else {
dev_dbg(hsdma->ddev.dev, "no desc to complete\n");
+ }
if (chan_issued)
set_bit(chan->id, >chan_issued);
@@ -456,8 +457,9 @@ static void mtk_hsdma_issue_pending(struct dma_chan *c)
if (gdma_next_desc(chan)) {
set_bit(chan->id, >chan_issued);
tasklet_schedule(>task);
- } else
+ } else {
dev_dbg(hsdma->ddev.dev, "no desc to issue\n");
+ }
}
spin_unlock_bh(>vchan.lock);
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
