diff options
author | Dexuan Cui <decui@microsoft.com> | 2017-01-28 13:46:02 -0500 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2017-01-31 04:59:48 -0500 |
commit | 433e19cf33d34bb6751c874a9c00980552fe508c (patch) | |
tree | ce6547ef2987fbb289fa28f03536328a42781651 | |
parent | 191e885a2e130e639bb0c8ee350d7047294f2ce6 (diff) |
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@microsoft.com>
Reported-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
Tested-by: Rolf Neugebauer <rolf.neugebauer@docker.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r-- | drivers/hv/ring_buffer.c | 1 | ||||
-rw-r--r-- | drivers/net/hyperv/netvsc.c | 6 | ||||
-rw-r--r-- | include/linux/hyperv.h | 32 |
3 files changed, 37 insertions, 2 deletions
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, | |||
383 | return ret; | 383 | return ret; |
384 | } | 384 | } |
385 | 385 | ||
386 | init_cached_read_index(channel); | ||
386 | next_read_location = hv_get_next_read_location(inring_info); | 387 | next_read_location = hv_get_next_read_location(inring_info); |
387 | next_read_location = hv_copyfrom_ringbuffer(inring_info, &desc, | 388 | next_read_location = hv_copyfrom_ringbuffer(inring_info, &desc, |
388 | sizeof(desc), | 389 | 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) | |||
1295 | ndev = hv_get_drvdata(device); | 1295 | ndev = hv_get_drvdata(device); |
1296 | buffer = get_per_channel_state(channel); | 1296 | buffer = get_per_channel_state(channel); |
1297 | 1297 | ||
1298 | /* commit_rd_index() -> hv_signal_on_read() needs this. */ | ||
1299 | init_cached_read_index(channel); | ||
1300 | |||
1298 | do { | 1301 | do { |
1299 | desc = get_next_pkt_raw(channel); | 1302 | desc = get_next_pkt_raw(channel); |
1300 | if (desc != NULL) { | 1303 | if (desc != NULL) { |
@@ -1347,6 +1350,9 @@ void netvsc_channel_cb(void *context) | |||
1347 | 1350 | ||
1348 | bufferlen = bytes_recvd; | 1351 | bufferlen = bytes_recvd; |
1349 | } | 1352 | } |
1353 | |||
1354 | init_cached_read_index(channel); | ||
1355 | |||
1350 | } while (1); | 1356 | } while (1); |
1351 | 1357 | ||
1352 | if (bufferlen > NETVSC_PACKET_SIZE) | 1358 | 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 { | |||
128 | u32 ring_data_startoffset; | 128 | u32 ring_data_startoffset; |
129 | u32 priv_write_index; | 129 | u32 priv_write_index; |
130 | u32 priv_read_index; | 130 | u32 priv_read_index; |
131 | u32 cached_read_index; | ||
131 | }; | 132 | }; |
132 | 133 | ||
133 | /* | 134 | /* |
@@ -180,6 +181,19 @@ static inline u32 hv_get_bytes_to_write(struct hv_ring_buffer_info *rbi) | |||
180 | return write; | 181 | return write; |
181 | } | 182 | } |
182 | 183 | ||
184 | static inline u32 hv_get_cached_bytes_to_write( | ||
185 | const struct hv_ring_buffer_info *rbi) | ||
186 | { | ||
187 | u32 read_loc, write_loc, dsize, write; | ||
188 | |||
189 | dsize = rbi->ring_datasize; | ||
190 | read_loc = rbi->cached_read_index; | ||
191 | write_loc = rbi->ring_buffer->write_index; | ||
192 | |||
193 | write = write_loc >= read_loc ? dsize - (write_loc - read_loc) : | ||
194 | read_loc - write_loc; | ||
195 | return write; | ||
196 | } | ||
183 | /* | 197 | /* |
184 | * VMBUS version is 32 bit entity broken up into | 198 | * VMBUS version is 32 bit entity broken up into |
185 | * two 16 bit quantities: major_number. minor_number. | 199 | * two 16 bit quantities: major_number. minor_number. |
@@ -1488,7 +1502,7 @@ hv_get_ring_buffer(struct hv_ring_buffer_info *ring_info) | |||
1488 | 1502 | ||
1489 | static inline void hv_signal_on_read(struct vmbus_channel *channel) | 1503 | static inline void hv_signal_on_read(struct vmbus_channel *channel) |
1490 | { | 1504 | { |
1491 | u32 cur_write_sz; | 1505 | u32 cur_write_sz, cached_write_sz; |
1492 | u32 pending_sz; | 1506 | u32 pending_sz; |
1493 | struct hv_ring_buffer_info *rbi = &channel->inbound; | 1507 | struct hv_ring_buffer_info *rbi = &channel->inbound; |
1494 | 1508 | ||
@@ -1512,12 +1526,24 @@ static inline void hv_signal_on_read(struct vmbus_channel *channel) | |||
1512 | 1526 | ||
1513 | cur_write_sz = hv_get_bytes_to_write(rbi); | 1527 | cur_write_sz = hv_get_bytes_to_write(rbi); |
1514 | 1528 | ||
1515 | if (cur_write_sz >= pending_sz) | 1529 | if (cur_write_sz < pending_sz) |
1530 | return; | ||
1531 | |||
1532 | cached_write_sz = hv_get_cached_bytes_to_write(rbi); | ||
1533 | if (cached_write_sz < pending_sz) | ||
1516 | vmbus_setevent(channel); | 1534 | vmbus_setevent(channel); |
1517 | 1535 | ||
1518 | return; | 1536 | return; |
1519 | } | 1537 | } |
1520 | 1538 | ||
1539 | static inline void | ||
1540 | init_cached_read_index(struct vmbus_channel *channel) | ||
1541 | { | ||
1542 | struct hv_ring_buffer_info *rbi = &channel->inbound; | ||
1543 | |||
1544 | rbi->cached_read_index = rbi->ring_buffer->read_index; | ||
1545 | } | ||
1546 | |||
1521 | /* | 1547 | /* |
1522 | * An API to support in-place processing of incoming VMBUS packets. | 1548 | * An API to support in-place processing of incoming VMBUS packets. |
1523 | */ | 1549 | */ |
@@ -1569,6 +1595,8 @@ static inline void put_pkt_raw(struct vmbus_channel *channel, | |||
1569 | * This call commits the read index and potentially signals the host. | 1595 | * This call commits the read index and potentially signals the host. |
1570 | * Here is the pattern for using the "in-place" consumption APIs: | 1596 | * Here is the pattern for using the "in-place" consumption APIs: |
1571 | * | 1597 | * |
1598 | * init_cached_read_index(); | ||
1599 | * | ||
1572 | * while (get_next_pkt_raw() { | 1600 | * while (get_next_pkt_raw() { |
1573 | * process the packet "in-place"; | 1601 | * process the packet "in-place"; |
1574 | * put_pkt_raw(); | 1602 | * put_pkt_raw(); |