* [PATCH net-next 0/2] netvsc: performance regressions in net-next
@ 2017-03-20 20:28 Stephen Hemminger
2017-03-20 20:28 ` [PATCH net-next 1/2] netvsc: fix NAPI performance regression Stephen Hemminger
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Stephen Hemminger @ 2017-03-20 20:28 UTC (permalink / raw)
To: kys, davem; +Cc: haiyangz, netdev, Stephen Hemminger
Fix for performance regression introduced with NAPI change;
and followup cleanup of vmbus core code.
Stephen Hemminger (2):
netvsc: fix NAPI performance regression
vmbus: no longer expose iterator internals
drivers/hv/ring_buffer.c | 51 +++++++++++++++++++++--------------------
drivers/net/hyperv/hyperv_net.h | 1 +
drivers/net/hyperv/netvsc.c | 41 +++++++++++++++------------------
include/linux/hyperv.h | 22 +-----------------
4 files changed, 46 insertions(+), 69 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 1/2] netvsc: fix NAPI performance regression
2017-03-20 20:28 [PATCH net-next 0/2] netvsc: performance regressions in net-next Stephen Hemminger
@ 2017-03-20 20:28 ` Stephen Hemminger
2017-03-20 20:28 ` [PATCH net-next 2/2] vmbus: no longer expose iterator internals Stephen Hemminger
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2017-03-20 20:28 UTC (permalink / raw)
To: kys, davem; +Cc: haiyangz, netdev, Stephen Hemminger
When using NAPI, the single stream performance declined signifcantly
because the poll routine was updating host after every burst
of packets. This excess signalling caused host throttling.
This fix restores the old behavior. Host is only signalled
after the ring has been emptied.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/net/hyperv/hyperv_net.h | 1 +
drivers/net/hyperv/netvsc.c | 41 ++++++++++++++++++-----------------------
2 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 6b5f75217694..a33f2ee86044 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -723,6 +723,7 @@ struct net_device_context {
/* Per channel data */
struct netvsc_channel {
struct vmbus_channel *channel;
+ const struct vmpacket_descriptor *desc;
struct napi_struct napi;
struct multi_send_data msd;
struct multi_recv_comp mrc;
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 989b7cd99380..727762d0f13b 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -1173,7 +1173,6 @@ static int netvsc_process_raw_pkt(struct hv_device *device,
struct vmbus_channel *channel,
struct netvsc_device *net_device,
struct net_device *ndev,
- u64 request_id,
const struct vmpacket_descriptor *desc)
{
struct net_device_context *net_device_ctx = netdev_priv(ndev);
@@ -1195,7 +1194,7 @@ static int netvsc_process_raw_pkt(struct hv_device *device,
default:
netdev_err(ndev, "unhandled packet type %d, tid %llx\n",
- desc->type, request_id);
+ desc->type, desc->trans_id);
break;
}
@@ -1222,28 +1221,20 @@ int netvsc_poll(struct napi_struct *napi, int budget)
u16 q_idx = channel->offermsg.offer.sub_channel_index;
struct net_device *ndev = hv_get_drvdata(device);
struct netvsc_device *net_device = net_device_to_netvsc_device(ndev);
- const struct vmpacket_descriptor *desc;
int work_done = 0;
- desc = hv_pkt_iter_first(channel);
- while (desc) {
- int count;
+ /* If starting a new interval */
+ if (!nvchan->desc)
+ nvchan->desc = hv_pkt_iter_first(channel);
- count = netvsc_process_raw_pkt(device, channel, net_device,
- ndev, desc->trans_id, desc);
- work_done += count;
- desc = __hv_pkt_iter_next(channel, desc);
-
- /* If receive packet budget is exhausted, reschedule */
- if (work_done >= budget) {
- work_done = budget;
- break;
- }
+ while (nvchan->desc && work_done < budget) {
+ work_done += netvsc_process_raw_pkt(device, channel, net_device,
+ ndev, nvchan->desc);
+ nvchan->desc = hv_pkt_iter_next(channel, nvchan->desc);
}
- hv_pkt_iter_close(channel);
- /* If budget was not exhausted and
- * not doing busy poll
+ /* If receive ring was exhausted
+ * and not doing busy poll
* then re-enable host interrupts
* and reschedule if ring is not empty.
*/
@@ -1253,7 +1244,9 @@ int netvsc_poll(struct napi_struct *napi, int budget)
napi_reschedule(napi);
netvsc_chk_recv_comp(net_device, channel, q_idx);
- return work_done;
+
+ /* Driver may overshoot since multiple packets per descriptor */
+ return min(work_done, budget);
}
/* Call back when data is available in host ring buffer.
@@ -1263,10 +1256,12 @@ void netvsc_channel_cb(void *context)
{
struct netvsc_channel *nvchan = context;
- /* disable interupts from host */
- hv_begin_read(&nvchan->channel->inbound);
+ if (napi_schedule_prep(&nvchan->napi)) {
+ /* disable interupts from host */
+ hv_begin_read(&nvchan->channel->inbound);
- napi_schedule(&nvchan->napi);
+ __napi_schedule(&nvchan->napi);
+ }
}
/*
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 2/2] vmbus: no longer expose iterator internals
2017-03-20 20:28 [PATCH net-next 0/2] netvsc: performance regressions in net-next Stephen Hemminger
2017-03-20 20:28 ` [PATCH net-next 1/2] netvsc: fix NAPI performance regression Stephen Hemminger
@ 2017-03-20 20:28 ` Stephen Hemminger
2017-03-20 20:31 ` [PATCH net-next 0/2] netvsc: performance regressions in net-next Stephen Hemminger
2017-03-22 18:22 ` David Miller
3 siblings, 0 replies; 6+ messages in thread
From: Stephen Hemminger @ 2017-03-20 20:28 UTC (permalink / raw)
To: kys, davem; +Cc: haiyangz, netdev, Stephen Hemminger
Since NAPI no longer needs to jump out of iterator early,
no need to expose the internal iterator steps.
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/hv/ring_buffer.c | 51 ++++++++++++++++++++++++------------------------
include/linux/hyperv.h | 22 +--------------------
2 files changed, 27 insertions(+), 46 deletions(-)
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index c3f1a9e33cef..280e2010913f 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -458,13 +458,31 @@ struct vmpacket_descriptor *hv_pkt_iter_first(struct vmbus_channel *channel)
EXPORT_SYMBOL_GPL(hv_pkt_iter_first);
/*
+ * Update host ring buffer after iterating over packets.
+ */
+static void hv_pkt_iter_close(struct vmbus_channel *channel)
+{
+ struct hv_ring_buffer_info *rbi = &channel->inbound;
+
+ /*
+ * Make sure all reads are done before we update the read index since
+ * the writer may start writing to the read area once the read index
+ * is updated.
+ */
+ virt_rmb();
+ rbi->ring_buffer->read_index = rbi->priv_read_index;
+
+ hv_signal_on_read(channel);
+}
+
+/*
* Get next vmbus packet from ring buffer.
*
* Advances the current location (priv_read_index) and checks for more
- * data. If the end of the ring buffer is reached, then return NULL.
+ * data. If at end of list, return NULL and update host.
*/
struct vmpacket_descriptor *
-__hv_pkt_iter_next(struct vmbus_channel *channel,
+hv_pkt_iter_next(struct vmbus_channel *channel,
const struct vmpacket_descriptor *desc)
{
struct hv_ring_buffer_info *rbi = &channel->inbound;
@@ -476,29 +494,12 @@ __hv_pkt_iter_next(struct vmbus_channel *channel,
if (rbi->priv_read_index >= dsize)
rbi->priv_read_index -= dsize;
- /* more data? */
- if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor))
+ /* if no more data? */
+ if (hv_pkt_iter_avail(rbi) < sizeof(struct vmpacket_descriptor)) {
+ hv_pkt_iter_close(channel);
return NULL;
- else
- return hv_get_ring_buffer(rbi) + rbi->priv_read_index;
-}
-EXPORT_SYMBOL_GPL(__hv_pkt_iter_next);
-
-/*
- * Update host ring buffer after iterating over packets.
- */
-void hv_pkt_iter_close(struct vmbus_channel *channel)
-{
- struct hv_ring_buffer_info *rbi = &channel->inbound;
-
- /*
- * Make sure all reads are done before we update the read index since
- * the writer may start writing to the read area once the read index
- * is updated.
- */
- virt_rmb();
- rbi->ring_buffer->read_index = rbi->priv_read_index;
+ }
- hv_signal_on_read(channel);
+ return hv_get_ring_buffer(rbi) + rbi->priv_read_index;
}
-EXPORT_SYMBOL_GPL(hv_pkt_iter_close);
+EXPORT_SYMBOL_GPL(hv_pkt_iter_next);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 36162485d663..7df6ab5b3067 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1550,32 +1550,12 @@ static inline u32 hv_pkt_datalen(const struct vmpacket_descriptor *desc)
return (desc->len8 << 3) - (desc->offset8 << 3);
}
-
struct vmpacket_descriptor *
hv_pkt_iter_first(struct vmbus_channel *channel);
struct vmpacket_descriptor *
-__hv_pkt_iter_next(struct vmbus_channel *channel,
- const struct vmpacket_descriptor *pkt);
-
-void hv_pkt_iter_close(struct vmbus_channel *channel);
-
-/*
- * Get next packet descriptor from iterator
- * If at end of list, return NULL and update host.
- */
-static inline struct vmpacket_descriptor *
hv_pkt_iter_next(struct vmbus_channel *channel,
- const struct vmpacket_descriptor *pkt)
-{
- struct vmpacket_descriptor *nxt;
-
- nxt = __hv_pkt_iter_next(channel, pkt);
- if (!nxt)
- hv_pkt_iter_close(channel);
-
- return nxt;
-}
+ const struct vmpacket_descriptor *pkt);
#define foreach_vmbus_pkt(pkt, channel) \
for (pkt = hv_pkt_iter_first(channel); pkt; \
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/2] netvsc: performance regressions in net-next
2017-03-20 20:28 [PATCH net-next 0/2] netvsc: performance regressions in net-next Stephen Hemminger
2017-03-20 20:28 ` [PATCH net-next 1/2] netvsc: fix NAPI performance regression Stephen Hemminger
2017-03-20 20:28 ` [PATCH net-next 2/2] vmbus: no longer expose iterator internals Stephen Hemminger
@ 2017-03-20 20:31 ` Stephen Hemminger
2017-03-22 18:23 ` David Miller
2017-03-22 18:22 ` David Miller
3 siblings, 1 reply; 6+ messages in thread
From: Stephen Hemminger @ 2017-03-20 20:31 UTC (permalink / raw)
To: kys, davem; +Cc: haiyangz, netdev, Stephen Hemminger
On Mon, 20 Mar 2017 13:28:03 -0700
Stephen Hemminger <stephen@networkplumber.org> wrote:
> Fix for performance regression introduced with NAPI change;
> and followup cleanup of vmbus core code.
>
> Stephen Hemminger (2):
> netvsc: fix NAPI performance regression
> vmbus: no longer expose iterator internals
>
> drivers/hv/ring_buffer.c | 51 +++++++++++++++++++++--------------------
> drivers/net/hyperv/hyperv_net.h | 1 +
> drivers/net/hyperv/netvsc.c | 41 +++++++++++++++------------------
> include/linux/hyperv.h | 22 +-----------------
> 4 files changed, 46 insertions(+), 69 deletions(-)
>
Drop this version. The code is correct the second patch causes
clashes with some later patches. Will resend.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/2] netvsc: performance regressions in net-next
2017-03-20 20:28 [PATCH net-next 0/2] netvsc: performance regressions in net-next Stephen Hemminger
` (2 preceding siblings ...)
2017-03-20 20:31 ` [PATCH net-next 0/2] netvsc: performance regressions in net-next Stephen Hemminger
@ 2017-03-22 18:22 ` David Miller
3 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-03-22 18:22 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, netdev, sthemmin
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 20 Mar 2017 13:28:03 -0700
> Fix for performance regression introduced with NAPI change;
> and followup cleanup of vmbus core code.
Series applied, thanks Stephen.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 0/2] netvsc: performance regressions in net-next
2017-03-20 20:31 ` [PATCH net-next 0/2] netvsc: performance regressions in net-next Stephen Hemminger
@ 2017-03-22 18:23 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-03-22 18:23 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, netdev, sthemmin
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Mon, 20 Mar 2017 13:31:52 -0700
> On Mon, 20 Mar 2017 13:28:03 -0700
> Stephen Hemminger <stephen@networkplumber.org> wrote:
>
>> Fix for performance regression introduced with NAPI change;
>> and followup cleanup of vmbus core code.
>>
>> Stephen Hemminger (2):
>> netvsc: fix NAPI performance regression
>> vmbus: no longer expose iterator internals
>>
>> drivers/hv/ring_buffer.c | 51 +++++++++++++++++++++--------------------
>> drivers/net/hyperv/hyperv_net.h | 1 +
>> drivers/net/hyperv/netvsc.c | 41 +++++++++++++++------------------
>> include/linux/hyperv.h | 22 +-----------------
>> 4 files changed, 46 insertions(+), 69 deletions(-)
>>
>
> Drop this version. The code is correct the second patch causes
> clashes with some later patches. Will resend.
Oh, ok, good thing I saw this before pushing out.
Reverted.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-22 18:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-20 20:28 [PATCH net-next 0/2] netvsc: performance regressions in net-next Stephen Hemminger
2017-03-20 20:28 ` [PATCH net-next 1/2] netvsc: fix NAPI performance regression Stephen Hemminger
2017-03-20 20:28 ` [PATCH net-next 2/2] vmbus: no longer expose iterator internals Stephen Hemminger
2017-03-20 20:31 ` [PATCH net-next 0/2] netvsc: performance regressions in net-next Stephen Hemminger
2017-03-22 18:23 ` David Miller
2017-03-22 18:22 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).