Re: FAILED: patch "[PATCH] Drivers: hv: vmbus: finally fix hv_need_to_signal_on_read()" failed to apply to 4.9-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



For this patch to apply, we also cherry-pick:
085b322dfe10 ("Drivers: hv: vmbus: Base host signaling strictly on the
ring state")
78777547745d ("Drivers: hv: vmbus: On write cleanup the logic to
interrupt the host")
983afbbd6d06 ("Drivers: hv: vmbus: On the read path cleanup the logic
to interrupt the host")

With these patches cherry-picked the patch applies cleanly and fixes
the original issue we have reported. Our kernel 4.9.8 tree with
patches applied is at
https://github.com/rneugeba/linux-stable/tree/v4.9.8-moby

Thanks
Rolf

On Sun, Feb 5, 2017 at 10:21 AM,  <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> The patch below does not apply to the 4.9-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@xxxxxxxxxxxxxxx>.
>
> thanks,
>
> greg k-h
>
> ------------------ original commit in Linus's tree ------------------
>
> From 433e19cf33d34bb6751c874a9c00980552fe508c Mon Sep 17 00:00:00 2001
> From: Dexuan Cui <decui@xxxxxxxxxxxxx>
> Date: Sat, 28 Jan 2017 11:46:02 -0700
> Subject: [PATCH] Drivers: hv: vmbus: finally fix hv_need_to_signal_on_read()
>
> Commit a389fcfd2cb5 ("Drivers: hv: vmbus: Fix signaling logic in
> hv_need_to_signal_on_read()")
> added the proper mb(), but removed the test "prev_write_sz < pending_sz"
> when making the signal decision.
>
> As a result, the guest can signal the host unnecessarily,
> and then the host can throttle the guest because the host
> thinks the guest is buggy or malicious; finally the user
> running stress test can perceive intermittent freeze of
> the guest.
>
> This patch brings back the test, and properly handles the
> in-place consumption APIs used by NetVSC (see get_next_pkt_raw(),
> put_pkt_raw() and commit_rd_index()).
>
> Fixes: a389fcfd2cb5 ("Drivers: hv: vmbus: Fix signaling logic in
> hv_need_to_signal_on_read()")
>
> Signed-off-by: Dexuan Cui <decui@xxxxxxxxxxxxx>
> Reported-by: Rolf Neugebauer <rolf.neugebauer@xxxxxxxxxx>
> Tested-by: Rolf Neugebauer <rolf.neugebauer@xxxxxxxxxx>
> Cc: "K. Y. Srinivasan" <kys@xxxxxxxxxxxxx>
> Cc: Haiyang Zhang <haiyangz@xxxxxxxxxxxxx>
> Cc: Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: K. Y. Srinivasan <kys@xxxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
> diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
> index cd49cb17eb7f..308dbda700eb 100644
> --- a/drivers/hv/ring_buffer.c
> +++ b/drivers/hv/ring_buffer.c
> @@ -383,6 +383,7 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
>                 return ret;
>         }
>
> +       init_cached_read_index(channel);
>         next_read_location = hv_get_next_read_location(inring_info);
>         next_read_location = hv_copyfrom_ringbuffer(inring_info, &desc,
>                                                     sizeof(desc),
> diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
> index 5a1cc089acb7..86e5749226ef 100644
> --- a/drivers/net/hyperv/netvsc.c
> +++ b/drivers/net/hyperv/netvsc.c
> @@ -1295,6 +1295,9 @@ void netvsc_channel_cb(void *context)
>         ndev = hv_get_drvdata(device);
>         buffer = get_per_channel_state(channel);
>
> +       /* commit_rd_index() -> hv_signal_on_read() needs this. */
> +       init_cached_read_index(channel);
> +
>         do {
>                 desc = get_next_pkt_raw(channel);
>                 if (desc != NULL) {
> @@ -1347,6 +1350,9 @@ void netvsc_channel_cb(void *context)
>
>                         bufferlen = bytes_recvd;
>                 }
> +
> +               init_cached_read_index(channel);
> +
>         } while (1);
>
>         if (bufferlen > NETVSC_PACKET_SIZE)
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 42fe43fb0c80..183efde54269 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -128,6 +128,7 @@ struct hv_ring_buffer_info {
>         u32 ring_data_startoffset;
>         u32 priv_write_index;
>         u32 priv_read_index;
> +       u32 cached_read_index;
>  };
>
>  /*
> @@ -180,6 +181,19 @@ static inline u32 hv_get_bytes_to_write(struct hv_ring_buffer_info *rbi)
>         return write;
>  }
>
> +static inline u32 hv_get_cached_bytes_to_write(
> +       const struct hv_ring_buffer_info *rbi)
> +{
> +       u32 read_loc, write_loc, dsize, write;
> +
> +       dsize = rbi->ring_datasize;
> +       read_loc = rbi->cached_read_index;
> +       write_loc = rbi->ring_buffer->write_index;
> +
> +       write = write_loc >= read_loc ? dsize - (write_loc - read_loc) :
> +               read_loc - write_loc;
> +       return write;
> +}
>  /*
>   * VMBUS version is 32 bit entity broken up into
>   * two 16 bit quantities: major_number. minor_number.
> @@ -1488,7 +1502,7 @@ hv_get_ring_buffer(struct hv_ring_buffer_info *ring_info)
>
>  static inline  void hv_signal_on_read(struct vmbus_channel *channel)
>  {
> -       u32 cur_write_sz;
> +       u32 cur_write_sz, cached_write_sz;
>         u32 pending_sz;
>         struct hv_ring_buffer_info *rbi = &channel->inbound;
>
> @@ -1512,12 +1526,24 @@ static inline  void hv_signal_on_read(struct vmbus_channel *channel)
>
>         cur_write_sz = hv_get_bytes_to_write(rbi);
>
> -       if (cur_write_sz >= pending_sz)
> +       if (cur_write_sz < pending_sz)
> +               return;
> +
> +       cached_write_sz = hv_get_cached_bytes_to_write(rbi);
> +       if (cached_write_sz < pending_sz)
>                 vmbus_setevent(channel);
>
>         return;
>  }
>
> +static inline void
> +init_cached_read_index(struct vmbus_channel *channel)
> +{
> +       struct hv_ring_buffer_info *rbi = &channel->inbound;
> +
> +       rbi->cached_read_index = rbi->ring_buffer->read_index;
> +}
> +
>  /*
>   * An API to support in-place processing of incoming VMBUS packets.
>   */
> @@ -1569,6 +1595,8 @@ static inline void put_pkt_raw(struct vmbus_channel *channel,
>   * This call commits the read index and potentially signals the host.
>   * Here is the pattern for using the "in-place" consumption APIs:
>   *
> + * init_cached_read_index();
> + *
>   * while (get_next_pkt_raw() {
>   *     process the packet "in-place";
>   *     put_pkt_raw();
>
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]