Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1419167 > unrolled thread

[PATCH 0/4] Drivers: hv: ring_buffer: make in-place consumption always possible

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] Drivers: hv: ring_buffer: make in-place consumption always possible Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-06-10 11:50 +0200
    [PATCH 3/4] Drivers: hv: ring_buffer: use wrap around mappings in hv_copy{from,to}_ringbuffer() Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-06-10 11:50 +0200
    [PATCH 2/4] Drivers: hv: ring_buffer: wrap around mappings for ring buffers Vitaly Kuznetsov <vkuznets@redhat.com> - 2016-06-10 11:50 +0200
    RE: [PATCH 0/4] Drivers: hv: ring_buffer: make in-place consumption  always possible KY Srinivasan <kys@microsoft.com> - 2016-06-10 15:50 +0200

#1419167 — [PATCH 0/4] Drivers: hv: ring_buffer: make in-place consumption always possible

K. Y.,

I'd like to accompany your netvsc performance improvement work by making
in-place consumption of VMBus packets always possible. Currently we forbid
it when a packet 'wraps around' the ring so we can't provide a single
pointer to it.

The idea if this series is dead simple: let's make a single virtual mapping
for two copies (actually, two sets of pages which consist the ring buffer)
of the ring buffer. With such a mapping we can always provide a pointers
for in-place consumption to drivers. Copy path can also benefit from such
mappings as we eliminate the need for conditional checking in copy_to/
copy_from functions and use a single memcpy().

Lightly tested with 'netvsc: Use the new in-place consumption APIs in the
rx path' patch and with storvsc driver.

Vitaly Kuznetsov (4):
  Drivers: hv: cleanup vmbus_open() for wrap around mappings
  Drivers: hv: ring_buffer: wrap around mappings for ring buffers
  Drivers: hv: ring_buffer: use wrap around mappings in
    hv_copy{from,to}_ringbuffer()
  Drivers: hv: ring_buffer: count on wrap around mappings in
    get_next_pkt_raw()

 drivers/hv/channel.c      | 68 ++++++++++++++++++++++++-----------------------
 drivers/hv/hyperv_vmbus.h |  4 +--
 drivers/hv/ring_buffer.c  | 61 ++++++++++++++++++++++++------------------
 include/linux/hyperv.h    | 32 ++++++++--------------
 4 files changed, 83 insertions(+), 82 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]

#1419169 — [PATCH 3/4] Drivers: hv: ring_buffer: use wrap around mappings in hv_copy{from,to}_ringbuffer()

With wrap around mappings for ring buffers we can always use a single
memcpy() to do the job.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/hv/ring_buffer.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index a84c45a..53e912a 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -164,18 +164,7 @@ static u32 hv_copyfrom_ringbuffer(
 	void *ring_buffer = hv_get_ring_buffer(ring_info);
 	u32 ring_buffer_size = hv_get_ring_buffersize(ring_info);
 
-	u32 frag_len;
-
-	/* wrap-around detected at the src */
-	if (destlen > ring_buffer_size - start_read_offset) {
-		frag_len = ring_buffer_size - start_read_offset;
-
-		memcpy(dest, ring_buffer + start_read_offset, frag_len);
-		memcpy(dest + frag_len, ring_buffer, destlen - frag_len);
-	} else
-
-		memcpy(dest, ring_buffer + start_read_offset, destlen);
-
+	memcpy(dest, ring_buffer + start_read_offset, destlen);
 
 	start_read_offset += destlen;
 	start_read_offset %= ring_buffer_size;
@@ -196,15 +185,8 @@ static u32 hv_copyto_ringbuffer(
 {
 	void *ring_buffer = hv_get_ring_buffer(ring_info);
 	u32 ring_buffer_size = hv_get_ring_buffersize(ring_info);
-	u32 frag_len;
-
-	/* wrap-around detected! */
-	if (srclen > ring_buffer_size - start_write_offset) {
-		frag_len = ring_buffer_size - start_write_offset;
-		memcpy(ring_buffer + start_write_offset, src, frag_len);
-		memcpy(ring_buffer, src + frag_len, srclen - frag_len);
-	} else
-		memcpy(ring_buffer + start_write_offset, src, srclen);
+
+	memcpy(ring_buffer + start_write_offset, src, srclen);
 
 	start_write_offset += srclen;
 	start_write_offset %= ring_buffer_size;
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1419175 — [PATCH 2/4] Drivers: hv: ring_buffer: wrap around mappings for ring buffers

Make it possible to always use a single memcpy() or to provide a direct
link to a packet on the ring buffer by creating virtual mapping for two
copies of the ring buffer with vmap(). Utilize currently empty
hv_ringbuffer_cleanup() to do the unmap.

While on it, replace sizeof(struct hv_ring_buffer) check
in hv_ringbuffer_init() with BUILD_BUG_ON() as it is a compile time check.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 drivers/hv/channel.c      | 29 ++++++++++++++---------------
 drivers/hv/hyperv_vmbus.h |  4 ++--
 drivers/hv/ring_buffer.c  | 39 +++++++++++++++++++++++++++++++++------
 3 files changed, 49 insertions(+), 23 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index a5a3734..a990a3f 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -70,7 +70,6 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 {
 	struct vmbus_channel_open_channel *open_msg;
 	struct vmbus_channel_msginfo *open_info = NULL;
-	void *in, *out;
 	unsigned long flags;
 	int ret, err = 0;
 	unsigned long t;
@@ -108,23 +107,21 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 		goto error_set_chnstate;
 	}
 
-	out = page_address(page);
-	in = (void *)((unsigned long)out + send_ringbuffer_size);
-
-	newchannel->ringbuffer_pages = out;
+	newchannel->ringbuffer_pages = page_address(page);
 	newchannel->ringbuffer_pagecount = (send_ringbuffer_size +
 					   recv_ringbuffer_size) >> PAGE_SHIFT;
 
-	ret = hv_ringbuffer_init(
-		&newchannel->outbound, out, send_ringbuffer_size);
+	ret = hv_ringbuffer_init(&newchannel->outbound, page,
+				 send_ringbuffer_size >> PAGE_SHIFT);
 
 	if (ret != 0) {
 		err = ret;
 		goto error_free_pages;
 	}
 
-	ret = hv_ringbuffer_init(
-		&newchannel->inbound, in, recv_ringbuffer_size);
+	ret = hv_ringbuffer_init(&newchannel->inbound,
+				 &page[send_ringbuffer_size >> PAGE_SHIFT],
+				 recv_ringbuffer_size >> PAGE_SHIFT);
 	if (ret != 0) {
 		err = ret;
 		goto error_free_pages;
@@ -135,10 +132,10 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size,
 	newchannel->ringbuffer_gpadlhandle = 0;
 
 	ret = vmbus_establish_gpadl(newchannel,
-					 newchannel->outbound.ring_buffer,
-					 send_ringbuffer_size +
-					 recv_ringbuffer_size,
-					 &newchannel->ringbuffer_gpadlhandle);
+				    page_address(page),
+				    send_ringbuffer_size +
+				    recv_ringbuffer_size,
+				    &newchannel->ringbuffer_gpadlhandle);
 
 	if (ret != 0) {
 		err = ret;
@@ -214,8 +211,10 @@ error_free_gpadl:
 	vmbus_teardown_gpadl(newchannel, newchannel->ringbuffer_gpadlhandle);
 	kfree(open_info);
 error_free_pages:
-	free_pages((unsigned long)out,
-		get_order(send_ringbuffer_size + recv_ringbuffer_size));
+	hv_ringbuffer_cleanup(&newchannel->outbound);
+	hv_ringbuffer_cleanup(&newchannel->inbound);
+	__free_pages(page,
+		     get_order(send_ringbuffer_size + recv_ringbuffer_size));
 error_set_chnstate:
 	newchannel->state = CHANNEL_OPEN_STATE;
 	return err;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 718b5c7..daaeb69 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -522,8 +522,8 @@ extern unsigned int host_info_edx;
 /* Interface */
 
 
-int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info, void *buffer,
-		   u32 buflen);
+int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
+		       struct page *pages, u32 pagecnt);
 
 void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info);
 
diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c
index fe586bf..a84c45a 100644
--- a/drivers/hv/ring_buffer.c
+++ b/drivers/hv/ring_buffer.c
@@ -27,6 +27,8 @@
 #include <linux/mm.h>
 #include <linux/hyperv.h>
 #include <linux/uio.h>
+#include <linux/vmalloc.h>
+#include <linux/slab.h>
 
 #include "hyperv_vmbus.h"
 
@@ -235,22 +237,46 @@ void hv_ringbuffer_get_debuginfo(struct hv_ring_buffer_info *ring_info,
 
 /* Initialize the ring buffer. */
 int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
-		   void *buffer, u32 buflen)
+		       struct page *pages, u32 page_cnt)
 {
-	if (sizeof(struct hv_ring_buffer) != PAGE_SIZE)
-		return -EINVAL;
+	int i;
+	struct page **pages_wraparound;
+
+	BUILD_BUG_ON((sizeof(struct hv_ring_buffer) != PAGE_SIZE));
 
 	memset(ring_info, 0, sizeof(struct hv_ring_buffer_info));
 
-	ring_info->ring_buffer = (struct hv_ring_buffer *)buffer;
+	/*
+	 * First page holds struct hv_ring_buffer, do wraparound mapping for
+	 * the rest.
+	 */
+	pages_wraparound = kzalloc(sizeof(struct page *) * (page_cnt * 2 - 1),
+				   GFP_KERNEL);
+	if (!pages_wraparound)
+		return -ENOMEM;
+
+	pages_wraparound[0] = pages;
+	for (i = 0; i < 2 * (page_cnt - 1); i++)
+		pages_wraparound[i + 1] = &pages[i % (page_cnt - 1) + 1];
+
+	ring_info->ring_buffer = (struct hv_ring_buffer *)
+		vmap(pages_wraparound, page_cnt * 2 - 1, VM_MAP, PAGE_KERNEL);
+
+	kfree(pages_wraparound);
+
+
+	if (!ring_info->ring_buffer)
+		return -ENOMEM;
+
 	ring_info->ring_buffer->read_index =
 		ring_info->ring_buffer->write_index = 0;
 
 	/* Set the feature bit for enabling flow control. */
 	ring_info->ring_buffer->feature_bits.value = 1;
 
-	ring_info->ring_size = buflen;
-	ring_info->ring_datasize = buflen - sizeof(struct hv_ring_buffer);
+	ring_info->ring_size = page_cnt << PAGE_SHIFT;
+	ring_info->ring_datasize = ring_info->ring_size -
+		sizeof(struct hv_ring_buffer);
 
 	spin_lock_init(&ring_info->ring_lock);
 
@@ -260,6 +286,7 @@ int hv_ringbuffer_init(struct hv_ring_buffer_info *ring_info,
 /* Cleanup the ring buffer. */
 void hv_ringbuffer_cleanup(struct hv_ring_buffer_info *ring_info)
 {
+	vunmap(ring_info->ring_buffer);
 }
 
 /* Write to the ring buffer. */
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1419439 — RE: [PATCH 0/4] Drivers: hv: ring_buffer: make in-place consumption always possible

> -----Original Message-----
> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Friday, June 10, 2016 2:47 AM
> To: devel@linuxdriverproject.org
> Cc: linux-kernel@vger.kernel.org; KY Srinivasan <kys@microsoft.com>; Haiyang
> Zhang <haiyangz@microsoft.com>
> Subject: [PATCH 0/4] Drivers: hv: ring_buffer: make in-place consumption
> always possible
> 
> K. Y.,
> 
> I'd like to accompany your netvsc performance improvement work by making
> in-place consumption of VMBus packets always possible. Currently we forbid
> it when a packet 'wraps around' the ring so we can't provide a single
> pointer to it.
> 
> The idea if this series is dead simple: let's make a single virtual mapping
> for two copies (actually, two sets of pages which consist the ring buffer)
> of the ring buffer. With such a mapping we can always provide a pointers
> for in-place consumption to drivers. Copy path can also benefit from such
> mappings as we eliminate the need for conditional checking in copy_to/
> copy_from functions and use a single memcpy().
> 
> Lightly tested with 'netvsc: Use the new in-place consumption APIs in the
> rx path' patch and with storvsc driver.

Thank you Vitaly. We will further test it.

K. Y
> 
> Vitaly Kuznetsov (4):
>   Drivers: hv: cleanup vmbus_open() for wrap around mappings
>   Drivers: hv: ring_buffer: wrap around mappings for ring buffers
>   Drivers: hv: ring_buffer: use wrap around mappings in
>     hv_copy{from,to}_ringbuffer()
>   Drivers: hv: ring_buffer: count on wrap around mappings in
>     get_next_pkt_raw()
> 
>  drivers/hv/channel.c      | 68 ++++++++++++++++++++++++-----------------------
>  drivers/hv/hyperv_vmbus.h |  4 +--
>  drivers/hv/ring_buffer.c  | 61 ++++++++++++++++++++++++------------------
>  include/linux/hyperv.h    | 32 ++++++++--------------
>  4 files changed, 83 insertions(+), 82 deletions(-)
> 
> --
> 2.5.5

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web