kernel-packages team mailing list archive

["Joshua R. Poulson" <jopoulso@xxxxxxxxxxxxx>]

Public bug reported:

The following patch has been submitted upstream in response to
investigation of customer issues with network connections hanging under
high load.

On the consumer side, we have interrupt driven flow management of the
producer. It is sufficient to base the signalling decision on the amount
of space that is available to write after the read is complete. The
current code samples the previous available space and uses this in
making the signalling decision. This state can be stale and is
unnecessary. Since the state can be stale, we end up not signalling the
host (when we should) and this can result in a hang. Fix this problem by
removing the unnecessary check.

I would like to thank Arseney Romanenko <arseneyr@xxxxxxxxxxxxx>
for pointing out this bug.

Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
Tested-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
---
 drivers/hv/ring_buffer.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index 5613e2b..085003a 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -103,8 +103,7 @@ static bool hv_need_to_signal(u32 old_write, struct hv_ring_buffer_info *rbi)
  *    there is room for the producer to send the pending packet.
  */

-static bool hv_need_to_signal_on_read(u32 prev_write_sz,
-                                     struct hv_ring_buffer_info *rbi)
+static bool hv_need_to_signal_on_read(struct hv_ring_buffer_info *rbi)
 {
        u32 cur_write_sz;
        u32 r_size;
@@ -120,7 +119,7 @@ static bool hv_need_to_signal_on_read(u32 prev_write_sz,
        cur_write_sz = write_loc >= read_loc ? r_size - (write_loc - read_loc) :
                        read_loc - write_loc;

-       if ((prev_write_sz < pending_sz) && (cur_write_sz >= pending_sz))
+       if (cur_write_sz >= pending_sz)
                return true;

        return false;
@@ -455,7 +454,7 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
        /* Update the read index */
        hv_set_next_read_location(inring_info, next_read_location);

-       *signal = hv_need_to_signal_on_read(bytes_avail_towrite, inring_info);
+       *signal = hv_need_to_signal_on_read(inring_info);

        return ret;
 }

We have customers who are encountering this issue. Although this patch
is not yet accepted upstream, we would like to get them a test kernel as
soon as we can.

** Affects: linux (Ubuntu)
     Importance: Undecided
         Status: Incomplete

-- 
You received this bug notification because you are a member of Kernel
Packages, which is subscribed to linux in Ubuntu.
https://bugs.launchpad.net/bugs/1556264

Title:
  [Hyper-V] vmbus: Fix a bug in hv_need_to_signal_on_read()

Status in linux package in Ubuntu:
  Incomplete

Bug description:
  The following patch has been submitted upstream in response to
  investigation of customer issues with network connections hanging
  under high load.

  On the consumer side, we have interrupt driven flow management of the
  producer. It is sufficient to base the signalling decision on the
  amount of space that is available to write after the read is complete.
  The current code samples the previous available space and uses this in
  making the signalling decision. This state can be stale and is
  unnecessary. Since the state can be stale, we end up not signalling
  the host (when we should) and this can result in a hang. Fix this
  problem by removing the unnecessary check.

  I would like to thank Arseney Romanenko <arseneyr@xxxxxxxxxxxxx>
  for pointing out this bug.

  Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
  Tested-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
  Cc: <stable@xxxxxxxxxxxxxxx>
  ---
   drivers/hv/ring_buffer.c |    7 +++----
   1 files changed, 3 insertions(+), 4 deletions(-)

  diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
  index 5613e2b..085003a 100644
  --- a/drivers/hv/ring_buffer.c
  +++ b/drivers/hv/ring_buffer.c
  @@ -103,8 +103,7 @@ static bool hv_need_to_signal(u32 old_write, struct hv_ring_buffer_info *rbi)
    *    there is room for the producer to send the pending packet.
    */

  -static bool hv_need_to_signal_on_read(u32 prev_write_sz,
  -                                     struct hv_ring_buffer_info *rbi)
  +static bool hv_need_to_signal_on_read(struct hv_ring_buffer_info *rbi)
   {
          u32 cur_write_sz;
          u32 r_size;
  @@ -120,7 +119,7 @@ static bool hv_need_to_signal_on_read(u32 prev_write_sz,
          cur_write_sz = write_loc >= read_loc ? r_size - (write_loc - read_loc) :
                          read_loc - write_loc;

  -       if ((prev_write_sz < pending_sz) && (cur_write_sz >= pending_sz))
  +       if (cur_write_sz >= pending_sz)
                  return true;

          return false;
  @@ -455,7 +454,7 @@ int hv_ringbuffer_read(struct hv_ring_buffer_info *inring_info,
          /* Update the read index */
          hv_set_next_read_location(inring_info, next_read_location);

  -       *signal = hv_need_to_signal_on_read(bytes_avail_towrite, inring_info);
  +       *signal = hv_need_to_signal_on_read(inring_info);

          return ret;
   }

  We have customers who are encountering this issue. Although this patch
  is not yet accepted upstream, we would like to get them a test kernel
  as soon as we can.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1556264/+subscriptions