All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
@ 2014-07-31  1:35 ` K. Y. Srinivasan
  0 siblings, 0 replies; 10+ messages in thread
From: K. Y. Srinivasan @ 2014-07-31  1:35 UTC (permalink / raw)
  To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang; +Cc: K. Y. Srinivasan

For forwarding scenarios, it will be useful to allocate larger
sendbuf. Make the necessary adjustments to permit this.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h |    2 +-
 drivers/net/hyperv/netvsc.c     |    7 ++-----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 6cc37c1..40ba1ef 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -584,7 +584,7 @@ struct nvsp_message {
 
 #define NETVSC_RECEIVE_BUFFER_SIZE		(1024*1024*16)	/* 16MB */
 #define NETVSC_RECEIVE_BUFFER_SIZE_LEGACY	(1024*1024*15)  /* 15MB */
-#define NETVSC_SEND_BUFFER_SIZE			(1024 * 1024)   /* 1MB */
+#define NETVSC_SEND_BUFFER_SIZE			(1024 * 1024 * 16)   /* 16MB */
 #define NETVSC_INVALID_INDEX			-1
 
 
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index c041f63..c76178e 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -193,8 +193,7 @@ static int netvsc_destroy_buf(struct netvsc_device *net_device)
 	}
 	if (net_device->send_buf) {
 		/* Free up the receive buffer */
-		free_pages((unsigned long)net_device->send_buf,
-			   get_order(net_device->send_buf_size));
+		vfree(net_device->send_buf);
 		net_device->send_buf = NULL;
 	}
 	kfree(net_device->send_section_map);
@@ -303,9 +302,7 @@ static int netvsc_init_buf(struct hv_device *device)
 
 	/* Now setup the send buffer.
 	 */
-	net_device->send_buf =
-		(void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO,
-					 get_order(net_device->send_buf_size));
+	net_device->send_buf = vzalloc(net_device->send_buf_size);
 	if (!net_device->send_buf) {
 		netdev_err(ndev, "unable to allocate send "
 			   "buffer of size %d\n", net_device->send_buf_size);
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
@ 2014-07-31  1:35 ` K. Y. Srinivasan
  0 siblings, 0 replies; 10+ messages in thread
From: K. Y. Srinivasan @ 2014-07-31  1:35 UTC (permalink / raw)
  To: davem, netdev, linux-kernel, devel, olaf, apw, jasowang

For forwarding scenarios, it will be useful to allocate larger
sendbuf. Make the necessary adjustments to permit this.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/net/hyperv/hyperv_net.h |    2 +-
 drivers/net/hyperv/netvsc.c     |    7 ++-----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index 6cc37c1..40ba1ef 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -584,7 +584,7 @@ struct nvsp_message {
 
 #define NETVSC_RECEIVE_BUFFER_SIZE		(1024*1024*16)	/* 16MB */
 #define NETVSC_RECEIVE_BUFFER_SIZE_LEGACY	(1024*1024*15)  /* 15MB */
-#define NETVSC_SEND_BUFFER_SIZE			(1024 * 1024)   /* 1MB */
+#define NETVSC_SEND_BUFFER_SIZE			(1024 * 1024 * 16)   /* 16MB */
 #define NETVSC_INVALID_INDEX			-1
 
 
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index c041f63..c76178e 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -193,8 +193,7 @@ static int netvsc_destroy_buf(struct netvsc_device *net_device)
 	}
 	if (net_device->send_buf) {
 		/* Free up the receive buffer */
-		free_pages((unsigned long)net_device->send_buf,
-			   get_order(net_device->send_buf_size));
+		vfree(net_device->send_buf);
 		net_device->send_buf = NULL;
 	}
 	kfree(net_device->send_section_map);
@@ -303,9 +302,7 @@ static int netvsc_init_buf(struct hv_device *device)
 
 	/* Now setup the send buffer.
 	 */
-	net_device->send_buf =
-		(void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO,
-					 get_order(net_device->send_buf_size));
+	net_device->send_buf = vzalloc(net_device->send_buf_size);
 	if (!net_device->send_buf) {
 		netdev_err(ndev, "unable to allocate send "
 			   "buffer of size %d\n", net_device->send_buf_size);
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
  2014-07-31  1:35 ` K. Y. Srinivasan
@ 2014-08-01  4:59   ` David Miller
  -1 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2014-08-01  4:59 UTC (permalink / raw)
  To: kys; +Cc: netdev, linux-kernel, devel, olaf, apw, jasowang

From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Wed, 30 Jul 2014 18:35:49 -0700

> For forwarding scenarios, it will be useful to allocate larger
> sendbuf. Make the necessary adjustments to permit this.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

This needs more information.

You're increasing the size by 16 times, 1MB --> 16MB, thus less
cache locality.

You're also now using vmalloc() memory, thus more TLB misses and
thrashing.

This must have a negative impact on performance, and you have to
test for that and quantify it when making a change as serious as
this one.

You also haven't gone into detail as to why forwarding scenerios
require more buffer space, than say thousands of local sockets
sending bulk TCP data.

I'm not applying this, it needs a lot more work.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
@ 2014-08-01  4:59   ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2014-08-01  4:59 UTC (permalink / raw)
  To: kys; +Cc: olaf, netdev, jasowang, linux-kernel, apw, devel

From: "K. Y. Srinivasan" <kys@microsoft.com>
Date: Wed, 30 Jul 2014 18:35:49 -0700

> For forwarding scenarios, it will be useful to allocate larger
> sendbuf. Make the necessary adjustments to permit this.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>

This needs more information.

You're increasing the size by 16 times, 1MB --> 16MB, thus less
cache locality.

You're also now using vmalloc() memory, thus more TLB misses and
thrashing.

This must have a negative impact on performance, and you have to
test for that and quantify it when making a change as serious as
this one.

You also haven't gone into detail as to why forwarding scenerios
require more buffer space, than say thousands of local sockets
sending bulk TCP data.

I'm not applying this, it needs a lot more work.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
  2014-08-01  4:59   ` David Miller
@ 2014-08-01 17:51     ` KY Srinivasan
  -1 siblings, 0 replies; 10+ messages in thread
From: KY Srinivasan @ 2014-08-01 17:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, devel, olaf, apw, jasowang



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, July 31, 2014 9:59 PM
> To: KY Srinivasan
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the
> sendbuf region
> 
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> Date: Wed, 30 Jul 2014 18:35:49 -0700
> 
> > For forwarding scenarios, it will be useful to allocate larger
> > sendbuf. Make the necessary adjustments to permit this.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> 
> This needs more information.
> 
> You're increasing the size by 16 times, 1MB --> 16MB, thus less cache locality.
> 
> You're also now using vmalloc() memory, thus more TLB misses and
> thrashing.
> 
> This must have a negative impact on performance, and you have to test for
> that and quantify it when making a change as serious as this one.
> 
> You also haven't gone into detail as to why forwarding scenerios require
> more buffer space, than say thousands of local sockets sending bulk TCP
> data.

David,

Intel did some benchmarking on our network throughput when Linux on Hyper-V was used as a gateway.
This fix gave us almost a 1 Gbps additional throughput on about 5Gbps base throughput we had prior to
Increasing the sendbuf size. The sendbuf mechanism is a copy based transport that we have which is clearly
more optimal than the copy-free page flipping mechanism (for small packets). In the forwarding scenario, we deal
only with MTU sized packets, and increasing the size of the senbuf area gave us the additional performance.

For what it is worth, Windows guests on Hyper-V, I am told use similar sendbuf size as well.

The exact value of sendbuf I think is less important than the fact that it needs to be larger than what Linux can
allocate as physically contiguous memory. Thus the change over to allocating via vmalloc(). As you know we currently
allocate 16MB receive buffer and we use vmalloc there for allocation. Also the low level channel code has already been
modified to deal with physically dis-contiguous memory in the ringbuffer setup.

Again based on experimentation Intel did, they say there was some improvement in throughput as the sendbuf size was
Increased up to 16MB and there was no effect on throughput beyond 16MB. Thus I chose 16MB here.

Increasing the sendbuf value makes a material difference in small packet handling. Let me know what I should do to
make this patch acceptable.

Regards,

K. Y

 
> 
> I'm not applying this, it needs a lot more work.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
@ 2014-08-01 17:51     ` KY Srinivasan
  0 siblings, 0 replies; 10+ messages in thread
From: KY Srinivasan @ 2014-08-01 17:51 UTC (permalink / raw)
  To: David Miller; +Cc: olaf, netdev, jasowang, linux-kernel, apw, devel



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Thursday, July 31, 2014 9:59 PM
> To: KY Srinivasan
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the
> sendbuf region
> 
> From: "K. Y. Srinivasan" <kys@microsoft.com>
> Date: Wed, 30 Jul 2014 18:35:49 -0700
> 
> > For forwarding scenarios, it will be useful to allocate larger
> > sendbuf. Make the necessary adjustments to permit this.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> 
> This needs more information.
> 
> You're increasing the size by 16 times, 1MB --> 16MB, thus less cache locality.
> 
> You're also now using vmalloc() memory, thus more TLB misses and
> thrashing.
> 
> This must have a negative impact on performance, and you have to test for
> that and quantify it when making a change as serious as this one.
> 
> You also haven't gone into detail as to why forwarding scenerios require
> more buffer space, than say thousands of local sockets sending bulk TCP
> data.

David,

Intel did some benchmarking on our network throughput when Linux on Hyper-V was used as a gateway.
This fix gave us almost a 1 Gbps additional throughput on about 5Gbps base throughput we had prior to
Increasing the sendbuf size. The sendbuf mechanism is a copy based transport that we have which is clearly
more optimal than the copy-free page flipping mechanism (for small packets). In the forwarding scenario, we deal
only with MTU sized packets, and increasing the size of the senbuf area gave us the additional performance.

For what it is worth, Windows guests on Hyper-V, I am told use similar sendbuf size as well.

The exact value of sendbuf I think is less important than the fact that it needs to be larger than what Linux can
allocate as physically contiguous memory. Thus the change over to allocating via vmalloc(). As you know we currently
allocate 16MB receive buffer and we use vmalloc there for allocation. Also the low level channel code has already been
modified to deal with physically dis-contiguous memory in the ringbuffer setup.

Again based on experimentation Intel did, they say there was some improvement in throughput as the sendbuf size was
Increased up to 16MB and there was no effect on throughput beyond 16MB. Thus I chose 16MB here.

Increasing the sendbuf value makes a material difference in small packet handling. Let me know what I should do to
make this patch acceptable.

Regards,

K. Y

 
> 
> I'm not applying this, it needs a lot more work.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
  2014-08-01 17:51     ` KY Srinivasan
@ 2014-08-02  6:14       ` David Miller
  -1 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2014-08-02  6:14 UTC (permalink / raw)
  To: kys; +Cc: netdev, linux-kernel, devel, olaf, apw, jasowang


Don't explain things to me in this thread.

Instead, tell the whole world and everyone who would ever see this
commit, in the commit log message.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
@ 2014-08-02  6:14       ` David Miller
  0 siblings, 0 replies; 10+ messages in thread
From: David Miller @ 2014-08-02  6:14 UTC (permalink / raw)
  To: kys; +Cc: olaf, netdev, jasowang, linux-kernel, apw, devel


Don't explain things to me in this thread.

Instead, tell the whole world and everyone who would ever see this
commit, in the commit log message.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
  2014-08-02  6:14       ` David Miller
@ 2014-08-02 16:18         ` KY Srinivasan
  -1 siblings, 0 replies; 10+ messages in thread
From: KY Srinivasan @ 2014-08-02 16:18 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-kernel, devel, olaf, apw, jasowang



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, August 1, 2014 11:14 PM
> To: KY Srinivasan
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the
> sendbuf region
> 
> 
> Don't explain things to me in this thread.
> 
> Instead, tell the whole world and everyone who would ever see this commit,
> in the commit log message.
Will do. Before I re-sent the patch with the explanation, I wanted to make sure I fully
understood your concerns.

Regards,

K. Y

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region
@ 2014-08-02 16:18         ` KY Srinivasan
  0 siblings, 0 replies; 10+ messages in thread
From: KY Srinivasan @ 2014-08-02 16:18 UTC (permalink / raw)
  To: David Miller; +Cc: olaf, netdev, jasowang, linux-kernel, apw, devel



> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Friday, August 1, 2014 11:14 PM
> To: KY Srinivasan
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the
> sendbuf region
> 
> 
> Don't explain things to me in this thread.
> 
> Instead, tell the whole world and everyone who would ever see this commit,
> in the commit log message.
Will do. Before I re-sent the patch with the explanation, I wanted to make sure I fully
understood your concerns.

Regards,

K. Y

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2014-08-02 16:19 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-31  1:35 [PATCH 1/1] Drivers: net-next: hyperv: Increase the size of the sendbuf region K. Y. Srinivasan
2014-07-31  1:35 ` K. Y. Srinivasan
2014-08-01  4:59 ` David Miller
2014-08-01  4:59   ` David Miller
2014-08-01 17:51   ` KY Srinivasan
2014-08-01 17:51     ` KY Srinivasan
2014-08-02  6:14     ` David Miller
2014-08-02  6:14       ` David Miller
2014-08-02 16:18       ` KY Srinivasan
2014-08-02 16:18         ` KY Srinivasan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.