[azure:x][PATCH v3 0/4] [Hyper-V] Drivers: hv: vmbus: Fix ring buffer signaling

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[azure:x][PATCH v3 0/4] [Hyper-V] Drivers: hv: vmbus: Fix ring buffer signaling

Marcelo Henrique Cerri
BugLink: http://bugs.launchpad.net/bugs/1748662

v3: fix commit message.
v2: revert additional sauce patches.

Marcelo Henrique Cerri (3):
  Revert "UBUNTU: SAUCE: vmbus: fix performance regression"
  Revert "UBUNTU: SAUCE: vmbus: simplify packet iterator"
  Revert "UBUNTU: SAUCE: vmbus: don't need to check interrupt mask on
    read side"

Michael Kelley (1):
  UBUNTU: SAUCE: hv: vmbus: Fix ring buffer signaling

 drivers/hv/ring_buffer.c | 64 +++++++++++++++++++++++++-----------------------
 include/linux/hyperv.h   |  2 +-
 2 files changed, 34 insertions(+), 32 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[azure:x][PATCH v3 1/4] Revert "UBUNTU: SAUCE: vmbus: fix performance regression"

Marcelo Henrique Cerri
BugLink: http://bugs.launchpad.net/bugs/1748662

This reverts commit 9ea3e22cb96dcca0d12213c12b503253ceb0e798.

Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/ring_buffer.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index dbc76554f9f9..8214042757cb 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -407,10 +407,10 @@ EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
 void hv_pkt_iter_close(struct vmbus_channel *channel)
 {
  struct hv_ring_buffer_info *rbi = &channel->inbound;
- u32 cached_write_sz, cur_write_sz, pending_sz;
+ u32 orig_write_sz;
 
  /* Available space before read_index update */
- cached_write_sz = hv_get_bytes_to_write(rbi);
+ orig_write_sz = hv_get_bytes_to_write(rbi);
 
  /*
  * Make sure all reads are done before we update the read index since
@@ -422,6 +422,10 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
  /* Update the position where ring buffer has been read from */
  rbi->ring_buffer->read_index = rbi->priv_read_index;
 
+ /* If more data is available then no need to signal */
+ if (hv_get_bytes_to_read(rbi))
+ return;
+
  /*
  * If the reading of the pend_sz were to be reordered and read
  * before we commit the new read index.
@@ -431,22 +435,18 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
  virt_wmb();
 
  if (rbi->ring_buffer->feature_bits.feat_pending_send_sz) {
- pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz);
-
- /* If the other end is not blocked on write don't bother. */
- if (pending_sz == 0)
- return;
-
- /* If pending write will not fit, don't give false hope. */
- cur_write_sz = hv_get_bytes_to_write(rbi);
- if (cur_write_sz < pending_sz)
- return;
+ u32 pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz);
 
  /*
  * If there was space before we began iteration, then
- * host was not blocked.
+ * host was not blocked. Also handles the case where
+ * pending_sz is zero because host has nothing pending.
  */
- if (cached_write_sz >= pending_sz)
+ if (orig_write_sz > pending_sz)
+ return;
+
+ /* If pending write will not fit, don't give false hope. */
+ if (hv_get_bytes_to_write(rbi) < pending_sz)
  return;
  }
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[azure:x][PATCH v3 2/4] Revert "UBUNTU: SAUCE: vmbus: simplify packet iterator"

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
BugLink: http://bugs.launchpad.net/bugs/1748662

This reverts commit 6e76d0d699a8effd941fa7732e18edf4173a6db0.

Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/ring_buffer.c | 42 ++++++++++++++++--------------------------
 include/linux/hyperv.h   |  2 +-
 2 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 8214042757cb..2f80666583c4 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -389,28 +389,18 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
  if (rbi->priv_read_index >= dsize)
  rbi->priv_read_index -= dsize;
 
+ /* more data? */
  return hv_pkt_iter_first(channel);
 }
 EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
 
 /*
  * Update host ring buffer after iterating over packets.
- *
- * Avoid unnecessary signaling of the host by making sure that all
- * data is read, and the host has not masked off the interrupt.
- *
- * In addition, in Windows 8 or later there is an extension for the
- * host to indicate how much space needs to be available before
- * signaling. The hos sets pending_send_sz to the number of bytes
- * that it is waiting to send.
  */
 void hv_pkt_iter_close(struct vmbus_channel *channel)
 {
  struct hv_ring_buffer_info *rbi = &channel->inbound;
- u32 orig_write_sz;
-
- /* Available space before read_index update */
- orig_write_sz = hv_get_bytes_to_write(rbi);
+ u32 orig_write_sz = hv_get_bytes_to_write(rbi);
 
  /*
  * Make sure all reads are done before we update the read index since
@@ -418,29 +408,29 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
  * is updated.
  */
  virt_rmb();
-
- /* Update the position where ring buffer has been read from */
  rbi->ring_buffer->read_index = rbi->priv_read_index;
 
- /* If more data is available then no need to signal */
- if (hv_get_bytes_to_read(rbi))
- return;
-
  /*
- * If the reading of the pend_sz were to be reordered and read
- * before we commit the new read index.
- * Then we could have if the host were to set the pending_sz
- * after we have already sampled pending_sz.
+ * Issue a full memory barrier before making the signaling decision.
+ * Here is the reason for having this barrier:
+ * If the reading of the pend_sz (in this function)
+ * were to be reordered and read before we commit the new read
+ * index (in the calling function)  we could
+ * have a problem. If the host were to set the pending_sz after we
+ * have sampled pending_sz and go to sleep before we commit the
+ * read index, we could miss sending the interrupt. Issue a full
+ * memory barrier to address this.
  */
- virt_wmb();
+ virt_mb();
 
  if (rbi->ring_buffer->feature_bits.feat_pending_send_sz) {
  u32 pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz);
 
  /*
- * If there was space before we began iteration, then
- * host was not blocked. Also handles the case where
- * pending_sz is zero because host has nothing pending.
+ * If there was space before we began iteration,
+ * then host was not blocked. Also handles case where
+ * pending_sz is zero then host has nothing pending
+ * and does not need to be signaled.
  */
  if (orig_write_sz > pending_sz)
  return;
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 822c7a3a4efc..dd3e59116ace 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -126,7 +126,7 @@ struct hv_ring_buffer_info {
  u32 ring_datasize; /* < ring_size */
  u32 ring_data_startoffset;
  u32 priv_write_index;
- u32 priv_read_index; /* read cursor */
+ u32 priv_read_index;
 };
 
 /*
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[azure:x][PATCH v3 3/4] Revert "UBUNTU: SAUCE: vmbus: don't need to check interrupt mask on read side"

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
BugLink: http://bugs.launchpad.net/bugs/1748662

This reverts commit 6bd427c166e666fa299c89081c7077f9aed81146.

Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/ring_buffer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 2f80666583c4..12eb8caa4263 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -423,6 +423,10 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
  */
  virt_mb();
 
+ /* If host has disabled notifications then skip */
+ if (rbi->ring_buffer->interrupt_mask)
+ return;
+
  if (rbi->ring_buffer->feature_bits.feat_pending_send_sz) {
  u32 pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz);
 
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[azure:x][PATCH v3 4/4] UBUNTU: SAUCE: hv: vmbus: Fix ring buffer signaling

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
From: Michael Kelley <[hidden email]>

BugLink: http://bugs.launchpad.net/bugs/1748662

Fix bugs in signaling the Hyper-V host when freeing space in the
host->guest ring buffer:

1. The interrupt_mask must not be used to determine whether to signal
   on the host->guest ring buffer
2. The ring buffer write_index must be read (via hv_get_bytes_to_write)
   *after* pending_send_sz is read in order to avoid a race condition
3. Comparisons with pending_send_sz must treat the "equals" case as
   not-enough-space
4. Don't signal if the pending_send_sz feature is not present. Older
   versions of Hyper-V that don't implement this feature will poll.

Fixes: 03bad714a161 ("vmbus: more host signalling avoidance")
Signed-off-by: Michael Kelley <[hidden email]>
Signed-off-by: Marcelo Henrique Cerri <[hidden email]>
---
 drivers/hv/ring_buffer.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 12eb8caa4263..c28217f4e8f2 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -400,7 +400,11 @@ EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
 void hv_pkt_iter_close(struct vmbus_channel *channel)
 {
  struct hv_ring_buffer_info *rbi = &channel->inbound;
- u32 orig_write_sz = hv_get_bytes_to_write(rbi);
+ u32 curr_write_sz;
+ u32 delta = rbi->ring_buffer->read_index < rbi->priv_read_index ?
+ (rbi->priv_read_index - rbi->ring_buffer->read_index) :
+ (rbi->ring_datasize - rbi->ring_buffer->read_index +
+ rbi->priv_read_index);
 
  /*
  * Make sure all reads are done before we update the read index since
@@ -423,27 +427,31 @@ void hv_pkt_iter_close(struct vmbus_channel *channel)
  */
  virt_mb();
 
- /* If host has disabled notifications then skip */
- if (rbi->ring_buffer->interrupt_mask)
- return;
-
  if (rbi->ring_buffer->feature_bits.feat_pending_send_sz) {
  u32 pending_sz = READ_ONCE(rbi->ring_buffer->pending_send_sz);
 
  /*
+ * Ensure the read of write_index in hv_get_bytes_to_write()
+ * happens after the read of pending_send_sz.
+ */
+ virt_rmb();
+ curr_write_sz = hv_get_bytes_to_write(rbi);
+
+ /*
  * If there was space before we began iteration,
  * then host was not blocked. Also handles case where
  * pending_sz is zero then host has nothing pending
  * and does not need to be signaled.
  */
- if (orig_write_sz > pending_sz)
+ if (curr_write_sz - delta > pending_sz)
  return;
 
  /* If pending write will not fit, don't give false hope. */
- if (hv_get_bytes_to_write(rbi) < pending_sz)
+ if (curr_write_sz <= pending_sz)
  return;
+
+ vmbus_setevent(channel);
  }
 
- vmbus_setevent(channel);
 }
 EXPORT_SYMBOL_GPL(hv_pkt_iter_close);
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [azure:x][PATCH v3 0/4] [Hyper-V] Drivers: hv: vmbus: Fix ring buffer signaling

Kamal Mostafa-2
In reply to this post by Marcelo Henrique Cerri
On Wed, Mar 07, 2018 at 12:38:44PM -0300, Marcelo Henrique Cerri wrote:

> BugLink: http://bugs.launchpad.net/bugs/1748662
>
> v3: fix commit message.
> v2: revert additional sauce patches.
>
> Marcelo Henrique Cerri (3):
>   Revert "UBUNTU: SAUCE: vmbus: fix performance regression"
>   Revert "UBUNTU: SAUCE: vmbus: simplify packet iterator"
>   Revert "UBUNTU: SAUCE: vmbus: don't need to check interrupt mask on
>     read side"
>
> Michael Kelley (1):
>   UBUNTU: SAUCE: hv: vmbus: Fix ring buffer signaling
>
>  drivers/hv/ring_buffer.c | 64 +++++++++++++++++++++++++-----------------------
>  include/linux/hyperv.h   |  2 +-
>  2 files changed, 34 insertions(+), 32 deletions(-)
>
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Acked-by: Kamal Mostafa <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [azure:x][PATCH v3 0/4] [Hyper-V] Drivers: hv: vmbus: Fix ring buffer signaling

Kleber Souza
In reply to this post by Marcelo Henrique Cerri
On 03/07/18 16:38, Marcelo Henrique Cerri wrote:

> BugLink: http://bugs.launchpad.net/bugs/1748662
>
> v3: fix commit message.
> v2: revert additional sauce patches.
>
> Marcelo Henrique Cerri (3):
>   Revert "UBUNTU: SAUCE: vmbus: fix performance regression"
>   Revert "UBUNTU: SAUCE: vmbus: simplify packet iterator"
>   Revert "UBUNTU: SAUCE: vmbus: don't need to check interrupt mask on
>     read side"
>
> Michael Kelley (1):
>   UBUNTU: SAUCE: hv: vmbus: Fix ring buffer signaling
>
>  drivers/hv/ring_buffer.c | 64 +++++++++++++++++++++++++-----------------------
>  include/linux/hyperv.h   |  2 +-
>  2 files changed, 34 insertions(+), 32 deletions(-)
>

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [azure:x][PATCH v3 0/4] [Hyper-V] Drivers: hv: vmbus: Fix ring buffer signaling

Marcelo Henrique Cerri
In reply to this post by Marcelo Henrique Cerri
--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (484 bytes) Download Attachment