* [PATCH V2 2/6] virtio/console: Add a failback for unstealable pipe buffer
From: Yoshihiro YUNOMAE @ 2012-08-09 12:30 UTC (permalink / raw)
To: linux-kernel
Cc: Herbert Xu, Arnd Bergmann, Frederic Weisbecker, qemu-devel,
Borislav Petkov, virtualization, Masami Hiramatsu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman, Amit Shah, yrl.pp-manager.tt
In-Reply-To: <20120809123029.8542.38311.stgit@ltc189.sdl.hitachi.co.jp>
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Add a failback memcpy path for unstealable pipe buffer.
If buf->ops->steal() fails, virtio-serial tries to
copy the page contents to an allocated page, instead
of just failing splice().
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
drivers/char/virtio_console.c | 28 +++++++++++++++++++++++++---
1 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 730816c..22b7373 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -794,7 +794,7 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
struct splice_desc *sd)
{
struct sg_list *sgl = sd->u.data;
- unsigned int len = 0;
+ unsigned int offset, len;
if (sgl->n == MAX_SPLICE_PAGES)
return 0;
@@ -807,9 +807,31 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
len = min(buf->len, sd->len);
sg_set_page(&(sgl->sg[sgl->n]), buf->page, len, buf->offset);
- sgl->n++;
- sgl->len += len;
+ } else {
+ /* Failback to copying a page */
+ struct page *page = alloc_page(GFP_KERNEL);
+ char *src = buf->ops->map(pipe, buf, 1);
+ char *dst;
+
+ if (!page)
+ return -ENOMEM;
+ dst = kmap(page);
+
+ offset = sd->pos & ~PAGE_MASK;
+
+ len = sd->len;
+ if (len + offset > PAGE_SIZE)
+ len = PAGE_SIZE - offset;
+
+ memcpy(dst + offset, src + buf->offset, len);
+
+ kunmap(page);
+ buf->ops->unmap(pipe, buf, src);
+
+ sg_set_page(&(sgl->sg[sgl->n]), page, len, offset);
}
+ sgl->n++;
+ sgl->len += len;
return len;
}
^ permalink raw reply related
* [PATCH V2 1/6] virtio/console: Add splice_write support
From: Yoshihiro YUNOMAE @ 2012-08-09 12:30 UTC (permalink / raw)
To: linux-kernel
Cc: Herbert Xu, Arnd Bergmann, Frederic Weisbecker, qemu-devel,
Borislav Petkov, virtualization, Masami Hiramatsu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman, Amit Shah, yrl.pp-manager.tt
In-Reply-To: <20120809123029.8542.38311.stgit@ltc189.sdl.hitachi.co.jp>
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Enable to use splice_write from pipe to virtio-console port.
This steals pages from pipe and directly send it to host.
Note that this may accelerate only the guest to host path.
Changes in v2:
- Use GFP_KERNEL instead of GFP_ATOMIC in syscall context function.
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
drivers/char/virtio_console.c | 136 +++++++++++++++++++++++++++++++++++++++--
1 files changed, 128 insertions(+), 8 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index cdf2f54..730816c 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -24,6 +24,8 @@
#include <linux/err.h>
#include <linux/freezer.h>
#include <linux/fs.h>
+#include <linux/splice.h>
+#include <linux/pagemap.h>
#include <linux/init.h>
#include <linux/list.h>
#include <linux/poll.h>
@@ -227,6 +229,7 @@ struct port {
bool guest_connected;
};
+#define MAX_SPLICE_PAGES 32
/* This is the very early arch-specified put chars function. */
static int (*early_put_chars)(u32, const char *, int);
@@ -474,26 +477,52 @@ static ssize_t send_control_msg(struct port *port, unsigned int event,
return 0;
}
+struct buffer_token {
+ union {
+ void *buf;
+ struct scatterlist *sg;
+ } u;
+ bool sgpages;
+};
+
+static void reclaim_sg_pages(struct scatterlist *sg)
+{
+ int i;
+ struct page *page;
+
+ for (i = 0; i < MAX_SPLICE_PAGES; i++) {
+ page = sg_page(&sg[i]);
+ if (!page)
+ break;
+ put_page(page);
+ }
+ kfree(sg);
+}
+
/* Callers must take the port->outvq_lock */
static void reclaim_consumed_buffers(struct port *port)
{
- void *buf;
+ struct buffer_token *tok;
unsigned int len;
if (!port->portdev) {
/* Device has been unplugged. vqs are already gone. */
return;
}
- while ((buf = virtqueue_get_buf(port->out_vq, &len))) {
- kfree(buf);
+ while ((tok = virtqueue_get_buf(port->out_vq, &len))) {
+ if (tok->sgpages)
+ reclaim_sg_pages(tok->u.sg);
+ else
+ kfree(tok->u.buf);
+ kfree(tok);
port->outvq_full = false;
}
}
-static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count,
- bool nonblock)
+static ssize_t __send_to_port(struct port *port, struct scatterlist *sg,
+ int nents, size_t in_count,
+ struct buffer_token *tok, bool nonblock)
{
- struct scatterlist sg[1];
struct virtqueue *out_vq;
ssize_t ret;
unsigned long flags;
@@ -505,8 +534,7 @@ static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count,
reclaim_consumed_buffers(port);
- sg_init_one(sg, in_buf, in_count);
- ret = virtqueue_add_buf(out_vq, sg, 1, 0, in_buf, GFP_ATOMIC);
+ ret = virtqueue_add_buf(out_vq, sg, nents, 0, tok, GFP_ATOMIC);
/* Tell Host to go! */
virtqueue_kick(out_vq);
@@ -544,6 +572,37 @@ done:
return in_count;
}
+static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count,
+ bool nonblock)
+{
+ struct scatterlist sg[1];
+ struct buffer_token *tok;
+
+ tok = kmalloc(sizeof(*tok), GFP_ATOMIC);
+ if (!tok)
+ return -ENOMEM;
+ tok->sgpages = false;
+ tok->u.buf = in_buf;
+
+ sg_init_one(sg, in_buf, in_count);
+
+ return __send_to_port(port, sg, 1, in_count, tok, nonblock);
+}
+
+static ssize_t send_pages(struct port *port, struct scatterlist *sg, int nents,
+ size_t in_count, bool nonblock)
+{
+ struct buffer_token *tok;
+
+ tok = kmalloc(sizeof(*tok), GFP_ATOMIC);
+ if (!tok)
+ return -ENOMEM;
+ tok->sgpages = true;
+ tok->u.sg = sg;
+
+ return __send_to_port(port, sg, nents, in_count, tok, nonblock);
+}
+
/*
* Give out the data that's requested from the buffer that we have
* queued up.
@@ -725,6 +784,66 @@ out:
return ret;
}
+struct sg_list {
+ unsigned int n;
+ size_t len;
+ struct scatterlist *sg;
+};
+
+static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
+ struct splice_desc *sd)
+{
+ struct sg_list *sgl = sd->u.data;
+ unsigned int len = 0;
+
+ if (sgl->n == MAX_SPLICE_PAGES)
+ return 0;
+
+ /* Try lock this page */
+ if (buf->ops->steal(pipe, buf) == 0) {
+ /* Get reference and unlock page for moving */
+ get_page(buf->page);
+ unlock_page(buf->page);
+
+ len = min(buf->len, sd->len);
+ sg_set_page(&(sgl->sg[sgl->n]), buf->page, len, buf->offset);
+ sgl->n++;
+ sgl->len += len;
+ }
+
+ return len;
+}
+
+/* Faster zero-copy write by splicing */
+static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
+ struct file *filp, loff_t *ppos,
+ size_t len, unsigned int flags)
+{
+ struct port *port = filp->private_data;
+ struct sg_list sgl;
+ ssize_t ret;
+ struct splice_desc sd = {
+ .total_len = len,
+ .flags = flags,
+ .pos = *ppos,
+ .u.data = &sgl,
+ };
+
+ sgl.n = 0;
+ sgl.len = 0;
+ sgl.sg = kmalloc(sizeof(struct scatterlist) * MAX_SPLICE_PAGES,
+ GFP_KERNEL);
+ if (unlikely(!sgl.sg))
+ return -ENOMEM;
+
+ sg_init_table(sgl.sg, MAX_SPLICE_PAGES);
+ ret = __splice_from_pipe(pipe, &sd, pipe_to_sg);
+ if (likely(ret > 0))
+ ret = send_pages(port, sgl.sg, sgl.n, sgl.len, true);
+
+ return ret;
+}
+
static unsigned int port_fops_poll(struct file *filp, poll_table *wait)
{
struct port *port;
@@ -856,6 +975,7 @@ static const struct file_operations port_fops = {
.open = port_fops_open,
.read = port_fops_read,
.write = port_fops_write,
+ .splice_write = port_fops_splice_write,
.poll = port_fops_poll,
.release = port_fops_release,
.fasync = port_fops_fasync,
^ permalink raw reply related
* [PATCH V2 0/6] virtio-trace: Support virtio-trace
From: Yoshihiro YUNOMAE @ 2012-08-09 12:30 UTC (permalink / raw)
To: linux-kernel
Cc: Herbert Xu, Arnd Bergmann, Frederic Weisbecker, qemu-devel,
Borislav Petkov, virtualization, Masami Hiramatsu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman, Amit Shah, yrl.pp-manager.tt
Hi All,
The following patch set provides a low-overhead system for collecting kernel
tracing data of guests by a host in a virtualization environment.
A guest OS generally shares some devices with other guests or a host, so
reasons of any problems occurring in a guest may be from other guests or a host.
Then, to collect some tracing data of a number of guests and a host is needed
when some problems occur in a virtualization environment. One of methods to
realize that is to collect tracing data of guests in a host. To do this, network
is generally used. However, high load will be taken to applications on guests
using network I/O because there are many network stack layers. Therefore,
a communication method for collecting the data without using network is needed.
We submitted a patch set of "IVRing", a ring-buffer driver constructed on
Inter-VM shared memory (IVShmem), to LKML http://lwn.net/Articles/500304/ in
this June. IVRing and the IVRing reader use POSIX shared memory each other
without using network, so a low-overhead system for collecting guest tracing
data is realized. However, this patch set has some problems as follows:
- use IVShmem instead of virtio
- create a new ring-buffer without using existing ring-buffer in kernel
- scalability
-- not support SMP environment
-- buffer size limitation
-- not support live migration (maybe difficult for realize this)
Therefore, we propose a new system "virtio-trace", which uses enhanced
virtio-serial and existing ring-buffer of ftrace, for collecting guest kernel
tracing data. In this system, there are 5 main components:
(1) Ring-buffer of ftrace in a guest
- When trace agent reads ring-buffer, a page is removed from ring-buffer.
(2) Trace agent in the guest
- Splice the page of ring-buffer to read_pipe using splice() without
memory copying. Then, the page is spliced from write_pipe to virtio
without memory copying.
(3) Virtio-console driver in the guest
- Pass the page to virtio-ring
(4) Virtio-serial bus in QEMU
- Copy the page to kernel pipe
(5) Reader in the host
- Read guest tracing data via FIFO(named pipe)
Here, this patch set is only for guest kernels, so guest and host don't need to
run the same kernel.
In this patch set, we cannot get text-formatted data but raw-formatted data on
a host. perf and trace-cmd can actually translate raw to text by using
information of kernel or trace format under tracing/events directory in
debugfs. In the same way, if the information of a guest is exported to a host,
we can translate raw data of a guest to text data on a host or a remote host.
We will use 9pfs to export that.
***Evaluation***
When a host collects tracing data of a guest, the performance of using
virtio-trace is compared with that of using native(just running ftrace),
IVRing, and virtio-serial(normal method of read/write).
<environment>
The overview of this evaluation is as follows:
(a) A guest on a KVM is prepared.
- The guest is dedicated one physical CPU as a virtual CPU(VCPU).
(b) The guest starts to write tracing data to ring-buffer of ftrace.
- The probe points are all trace points of sched, timer, and kmem.
(c) Writing trace data, dhrystone 2 in UNIX bench is executed as a benchmark
tool in the guest.
- Dhrystone 2 intends system performance by repeating integer arithmetic
as a score.
- Since higher score equals to better system performance, if the score
decrease based on bare environment, it indicates that any operation
disturbs the integer arithmetic. Then, we define the overhead of
transporting trace data is calculated as follows:
OVERHEAD = (1 - SCORE_OF_A_METHOD/NATIVE_SCORE) * 100.
The performance of each method is compared as follows:
[1] Native
- only recording trace data to ring-buffer on a guest
[2] Virtio-trace
- running a trace agent on a guest
- a reader on a host opens FIFO using cat command
[3] IVRing
- A SystemTap script in a guest records trace data to IVRing.
-- probe points are same as ftrace.
[4] Virtio-serial(normal)
- A reader(using cat) on a guest output trace data to a host using
standard output via virtio-serial.
Other information is as follows:
- host
kernel: 3.3.7-1 (Fedora16)
CPU: Intel Xeon x5660@2.80GHz(12core)
Memory: 48GB
- guest(only booting one guest)
kernel: 3.5.0-rc4+ (Fedora16)
CPU: 1VCPU(dedicated)
Memory: 1GB
<result>
3 patterns based on the bare environment were indicated as follows:
Scores overhead against [0] Native
[0] Native: 28807569.5 -
[1] Virtio-trace: 28685049.5 0.43%
[2] IVRing: 28418595.5 1.35%
[3] Virtio-serial: 13262258.7 53.96%
***Just enhancement ideas***
- Support for trace-cmd
- Support for 9pfs protocol
- Support for non-blocking mode in QEMU
v2:
- Use GFP_KERNEL instead of GFP_ATOMIC in syscall context function in 1/6
- Just a minor fix for avoiding a confliction with previous patch in 5/6
- Cleanup (change fprintf() to pr_err() and an include guard) in 6/6
Thank you,
---
Masami Hiramatsu (5):
virtio/console: Allocate scatterlist according to the current pipe size
ftrace: Allow stealing pages from pipe buffer
virtio/console: Wait until the port is ready on splice
virtio/console: Add a failback for unstealable pipe buffer
virtio/console: Add splice_write support
Yoshihiro YUNOMAE (1):
tools: Add guest trace agent as a user tool
drivers/char/virtio_console.c | 198 ++++++++++++++++++--
kernel/trace/trace.c | 8 -
tools/virtio/virtio-trace/Makefile | 14 +
tools/virtio/virtio-trace/README | 118 ++++++++++++
tools/virtio/virtio-trace/trace-agent-ctl.c | 137 ++++++++++++++
tools/virtio/virtio-trace/trace-agent-rw.c | 192 +++++++++++++++++++
tools/virtio/virtio-trace/trace-agent.c | 270 +++++++++++++++++++++++++++
tools/virtio/virtio-trace/trace-agent.h | 75 ++++++++
8 files changed, 985 insertions(+), 27 deletions(-)
create mode 100644 tools/virtio/virtio-trace/Makefile
create mode 100644 tools/virtio/virtio-trace/README
create mode 100644 tools/virtio/virtio-trace/trace-agent-ctl.c
create mode 100644 tools/virtio/virtio-trace/trace-agent-rw.c
create mode 100644 tools/virtio/virtio-trace/trace-agent.c
create mode 100644 tools/virtio/virtio-trace/trace-agent.h
--
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@hitachi.com
^ permalink raw reply
* Re: [RFC PATCH 0/6] virtio-trace: Support virtio-trace
From: Amit Shah @ 2012-08-09 10:16 UTC (permalink / raw)
To: Yoshihiro YUNOMAE
Cc: Arnd Bergmann, qemu-devel, Frederic Weisbecker, yrl.pp-manager.tt,
linux-kernel, Borislav Petkov, virtualization, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman
In-Reply-To: <20120724023657.6600.52706.stgit@ltc189.sdl.hitachi.co.jp>
Hi,
On (Tue) 24 Jul 2012 [11:36:57], Yoshihiro YUNOMAE wrote:
> Hi All,
>
> The following patch set provides a low-overhead system for collecting kernel
> tracing data of guests by a host in a virtualization environment.
So I just have one minor comment, please post a non-RFC version of the
patch.
Since you have an ACK from Steven for the ftrace patch, I guess Rusty
can push this in via his virtio tree?
I'll ack the virtio-console bits in the next series you send.
Thanks,
Amit
^ permalink raw reply
* Re: [RFC PATCH 2/6] virtio/console: Add a failback for unstealable pipe buffer
From: Amit Shah @ 2012-08-09 10:14 UTC (permalink / raw)
To: Avi Kivity
Cc: Arnd Bergmann, qemu-devel, Frederic Weisbecker, yrl.pp-manager.tt,
linux-kernel, Borislav Petkov, virtualization, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman, Masami Hiramatsu
In-Reply-To: <502389B5.4020506@redhat.com>
On (Thu) 09 Aug 2012 [12:58:13], Avi Kivity wrote:
> On 08/09/2012 12:55 PM, Amit Shah wrote:
> > On (Thu) 09 Aug 2012 [18:24:58], Masami Hiramatsu wrote:
> >> (2012/08/09 18:03), Amit Shah wrote:
> >> > On (Tue) 24 Jul 2012 [11:37:18], Yoshihiro YUNOMAE wrote:
> >> >> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >> >>
> >> >> Add a failback memcpy path for unstealable pipe buffer.
> >> >> If buf->ops->steal() fails, virtio-serial tries to
> >> >> copy the page contents to an allocated page, instead
> >> >> of just failing splice().
> >> >>
> >> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >> >> Cc: Amit Shah <amit.shah@redhat.com>
> >> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> >> ---
> >> >>
> >> >> drivers/char/virtio_console.c | 28 +++++++++++++++++++++++++---
> >> >> 1 files changed, 25 insertions(+), 3 deletions(-)
> >> >>
> >> >> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> >> >> index fe31b2f..911cb3e 100644
> >> >> --- a/drivers/char/virtio_console.c
> >> >> +++ b/drivers/char/virtio_console.c
> >> >> @@ -794,7 +794,7 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
> >> >> struct splice_desc *sd)
> >> >> {
> >> >> struct sg_list *sgl = sd->u.data;
> >> >> - unsigned int len = 0;
> >> >> + unsigned int offset, len;
> >> >>
> >> >> if (sgl->n == MAX_SPLICE_PAGES)
> >> >> return 0;
> >> >> @@ -807,9 +807,31 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
> >> >>
> >> >> len = min(buf->len, sd->len);
> >> >> sg_set_page(&(sgl->sg[sgl->n]), buf->page, len, buf->offset);
> >> >> - sgl->n++;
> >> >> - sgl->len += len;
> >> >> + } else {
> >> >> + /* Failback to copying a page */
> >> >> + struct page *page = alloc_page(GFP_KERNEL);
> >> >
> >> > I prefer zeroing out the page. If there's not enough data to be
> >> > filled in the page, the remaining data can be leaked to the host.
> >>
> >> Yeah, it is really easy to fix that.
> >> But out of curiosity, would that be really a problem?
> >> I guess that host can access any guest page if need. If that
> >> is right, is that really insecure to leak randomly allocated
> >> unused page to the host?
> >
> > I'm not sure if there is a way to really attack, but just something I
> > had thought about: the host kernel can access any guest page, that's
> > not something we can prevent.
> >
> > However, if qemu is restricted from accessing guest pages, and the
> > guest shares this page with qemu for r/w purposes via the virtio
> > channel, a qemu exploit can expose guest data to host userspace.
> >
> > I agree this is completely theoretical; can someone else with more
> > insight confirm or deny my apprehensions?
>
> qemu can read and write any guest page (for the guest it controls).
OK, thanks for confirming -- no need to change this patch, then.
Amit
^ permalink raw reply
* Re: [RFC PATCH 2/6] virtio/console: Add a failback for unstealable pipe buffer
From: Avi Kivity @ 2012-08-09 9:58 UTC (permalink / raw)
To: Amit Shah
Cc: Arnd Bergmann, qemu-devel, Frederic Weisbecker, yrl.pp-manager.tt,
linux-kernel, Borislav Petkov, virtualization, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman, Masami Hiramatsu
In-Reply-To: <20120809095514.GJ3280@amit.redhat.com>
On 08/09/2012 12:55 PM, Amit Shah wrote:
> On (Thu) 09 Aug 2012 [18:24:58], Masami Hiramatsu wrote:
>> (2012/08/09 18:03), Amit Shah wrote:
>> > On (Tue) 24 Jul 2012 [11:37:18], Yoshihiro YUNOMAE wrote:
>> >> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> >>
>> >> Add a failback memcpy path for unstealable pipe buffer.
>> >> If buf->ops->steal() fails, virtio-serial tries to
>> >> copy the page contents to an allocated page, instead
>> >> of just failing splice().
>> >>
>> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> >> Cc: Amit Shah <amit.shah@redhat.com>
>> >> Cc: Arnd Bergmann <arnd@arndb.de>
>> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> >> ---
>> >>
>> >> drivers/char/virtio_console.c | 28 +++++++++++++++++++++++++---
>> >> 1 files changed, 25 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
>> >> index fe31b2f..911cb3e 100644
>> >> --- a/drivers/char/virtio_console.c
>> >> +++ b/drivers/char/virtio_console.c
>> >> @@ -794,7 +794,7 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>> >> struct splice_desc *sd)
>> >> {
>> >> struct sg_list *sgl = sd->u.data;
>> >> - unsigned int len = 0;
>> >> + unsigned int offset, len;
>> >>
>> >> if (sgl->n == MAX_SPLICE_PAGES)
>> >> return 0;
>> >> @@ -807,9 +807,31 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>> >>
>> >> len = min(buf->len, sd->len);
>> >> sg_set_page(&(sgl->sg[sgl->n]), buf->page, len, buf->offset);
>> >> - sgl->n++;
>> >> - sgl->len += len;
>> >> + } else {
>> >> + /* Failback to copying a page */
>> >> + struct page *page = alloc_page(GFP_KERNEL);
>> >
>> > I prefer zeroing out the page. If there's not enough data to be
>> > filled in the page, the remaining data can be leaked to the host.
>>
>> Yeah, it is really easy to fix that.
>> But out of curiosity, would that be really a problem?
>> I guess that host can access any guest page if need. If that
>> is right, is that really insecure to leak randomly allocated
>> unused page to the host?
>
> I'm not sure if there is a way to really attack, but just something I
> had thought about: the host kernel can access any guest page, that's
> not something we can prevent.
>
> However, if qemu is restricted from accessing guest pages, and the
> guest shares this page with qemu for r/w purposes via the virtio
> channel, a qemu exploit can expose guest data to host userspace.
>
> I agree this is completely theoretical; can someone else with more
> insight confirm or deny my apprehensions?
qemu can read and write any guest page (for the guest it controls).
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [RFC PATCH 2/6] virtio/console: Add a failback for unstealable pipe buffer
From: Amit Shah @ 2012-08-09 9:55 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Arnd Bergmann, qemu-devel, Frederic Weisbecker, yrl.pp-manager.tt,
linux-kernel, Borislav Petkov, virtualization, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman, Avi Kivity
In-Reply-To: <502381EA.80805@hitachi.com>
On (Thu) 09 Aug 2012 [18:24:58], Masami Hiramatsu wrote:
> (2012/08/09 18:03), Amit Shah wrote:
> > On (Tue) 24 Jul 2012 [11:37:18], Yoshihiro YUNOMAE wrote:
> >> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >>
> >> Add a failback memcpy path for unstealable pipe buffer.
> >> If buf->ops->steal() fails, virtio-serial tries to
> >> copy the page contents to an allocated page, instead
> >> of just failing splice().
> >>
> >> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> >> Cc: Amit Shah <amit.shah@redhat.com>
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> ---
> >>
> >> drivers/char/virtio_console.c | 28 +++++++++++++++++++++++++---
> >> 1 files changed, 25 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> >> index fe31b2f..911cb3e 100644
> >> --- a/drivers/char/virtio_console.c
> >> +++ b/drivers/char/virtio_console.c
> >> @@ -794,7 +794,7 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
> >> struct splice_desc *sd)
> >> {
> >> struct sg_list *sgl = sd->u.data;
> >> - unsigned int len = 0;
> >> + unsigned int offset, len;
> >>
> >> if (sgl->n == MAX_SPLICE_PAGES)
> >> return 0;
> >> @@ -807,9 +807,31 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
> >>
> >> len = min(buf->len, sd->len);
> >> sg_set_page(&(sgl->sg[sgl->n]), buf->page, len, buf->offset);
> >> - sgl->n++;
> >> - sgl->len += len;
> >> + } else {
> >> + /* Failback to copying a page */
> >> + struct page *page = alloc_page(GFP_KERNEL);
> >
> > I prefer zeroing out the page. If there's not enough data to be
> > filled in the page, the remaining data can be leaked to the host.
>
> Yeah, it is really easy to fix that.
> But out of curiosity, would that be really a problem?
> I guess that host can access any guest page if need. If that
> is right, is that really insecure to leak randomly allocated
> unused page to the host?
I'm not sure if there is a way to really attack, but just something I
had thought about: the host kernel can access any guest page, that's
not something we can prevent.
However, if qemu is restricted from accessing guest pages, and the
guest shares this page with qemu for r/w purposes via the virtio
channel, a qemu exploit can expose guest data to host userspace.
I agree this is completely theoretical; can someone else with more
insight confirm or deny my apprehensions?
Amit
^ permalink raw reply
* Re: [RFC PATCH 2/6] virtio/console: Add a failback for unstealable pipe buffer
From: Masami Hiramatsu @ 2012-08-09 9:24 UTC (permalink / raw)
To: Amit Shah
Cc: Arnd Bergmann, qemu-devel, Frederic Weisbecker, yrl.pp-manager.tt,
linux-kernel, Borislav Petkov, virtualization, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman
In-Reply-To: <20120809090312.GH3280@amit.redhat.com>
(2012/08/09 18:03), Amit Shah wrote:
> On (Tue) 24 Jul 2012 [11:37:18], Yoshihiro YUNOMAE wrote:
>> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>
>> Add a failback memcpy path for unstealable pipe buffer.
>> If buf->ops->steal() fails, virtio-serial tries to
>> copy the page contents to an allocated page, instead
>> of just failing splice().
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Amit Shah <amit.shah@redhat.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> ---
>>
>> drivers/char/virtio_console.c | 28 +++++++++++++++++++++++++---
>> 1 files changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
>> index fe31b2f..911cb3e 100644
>> --- a/drivers/char/virtio_console.c
>> +++ b/drivers/char/virtio_console.c
>> @@ -794,7 +794,7 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>> struct splice_desc *sd)
>> {
>> struct sg_list *sgl = sd->u.data;
>> - unsigned int len = 0;
>> + unsigned int offset, len;
>>
>> if (sgl->n == MAX_SPLICE_PAGES)
>> return 0;
>> @@ -807,9 +807,31 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>>
>> len = min(buf->len, sd->len);
>> sg_set_page(&(sgl->sg[sgl->n]), buf->page, len, buf->offset);
>> - sgl->n++;
>> - sgl->len += len;
>> + } else {
>> + /* Failback to copying a page */
>> + struct page *page = alloc_page(GFP_KERNEL);
>
> I prefer zeroing out the page. If there's not enough data to be
> filled in the page, the remaining data can be leaked to the host.
Yeah, it is really easy to fix that.
But out of curiosity, would that be really a problem?
I guess that host can access any guest page if need. If that
is right, is that really insecure to leak randomly allocated
unused page to the host?
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply
* Re: [RFC PATCH 2/6] virtio/console: Add a failback for unstealable pipe buffer
From: Borislav Petkov @ 2012-08-09 9:24 UTC (permalink / raw)
To: Amit Shah
Cc: Arnd Bergmann, qemu-devel, Frederic Weisbecker, yrl.pp-manager.tt,
linux-kernel, Borislav Petkov, virtualization, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman, Masami Hiramatsu
In-Reply-To: <20120809090312.GH3280@amit.redhat.com>
On Thu, Aug 09, 2012 at 02:33:12PM +0530, Amit Shah wrote:
> > @@ -807,9 +807,31 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
> >
> > len = min(buf->len, sd->len);
> > sg_set_page(&(sgl->sg[sgl->n]), buf->page, len, buf->offset);
> > - sgl->n++;
> > - sgl->len += len;
> > + } else {
> > + /* Failback to copying a page */
> > + struct page *page = alloc_page(GFP_KERNEL);
>
> I prefer zeroing out the page. If there's not enough data to be
> filled in the page, the remaining data can be leaked to the host.
get_zeroed_page()?
--
Regards/Gruss,
Boris.
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551
^ permalink raw reply
* Re: Re: [RFC PATCH 1/6] virtio/console: Add splice_write support
From: Masami Hiramatsu @ 2012-08-09 9:12 UTC (permalink / raw)
To: Amit Shah
Cc: Arnd Bergmann, qemu-devel, Frederic Weisbecker, yrl.pp-manager.tt,
linux-kernel, Borislav Petkov, virtualization, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman
In-Reply-To: <20120809090015.GG3280@amit.redhat.com>
(2012/08/09 18:00), Amit Shah wrote:
> On (Tue) 24 Jul 2012 [11:37:07], Yoshihiro YUNOMAE wrote:
>> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>>
>> Enable to use splice_write from pipe to virtio-console port.
>> This steals pages from pipe and directly send it to host.
>>
>> Note that this may accelerate only the guest to host path.
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> Cc: Amit Shah <amit.shah@redhat.com>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> ---
>
>> +/* Faster zero-copy write by splicing */
>> +static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
>> + struct file *filp, loff_t *ppos,
>> + size_t len, unsigned int flags)
>> +{
>> + struct port *port = filp->private_data;
>> + struct sg_list sgl;
>> + ssize_t ret;
>> + struct splice_desc sd = {
>> + .total_len = len,
>> + .flags = flags,
>> + .pos = *ppos,
>> + .u.data = &sgl,
>> + };
>> +
>> + sgl.n = 0;
>> + sgl.len = 0;
>> + sgl.sg = kmalloc(sizeof(struct scatterlist) * MAX_SPLICE_PAGES,
>> + GFP_ATOMIC);
>
> Do you expect this function to be called from interrupt context?
No, not at all. Oops, that should be GFP_KERNEL...
Thank you for pointing it out.
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply
* Re: [RFC PATCH 2/6] virtio/console: Add a failback for unstealable pipe buffer
From: Amit Shah @ 2012-08-09 9:03 UTC (permalink / raw)
To: Yoshihiro YUNOMAE
Cc: Arnd Bergmann, qemu-devel, Frederic Weisbecker, yrl.pp-manager.tt,
linux-kernel, Borislav Petkov, virtualization, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman, Masami Hiramatsu
In-Reply-To: <20120724023718.6600.68836.stgit@ltc189.sdl.hitachi.co.jp>
On (Tue) 24 Jul 2012 [11:37:18], Yoshihiro YUNOMAE wrote:
> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>
> Add a failback memcpy path for unstealable pipe buffer.
> If buf->ops->steal() fails, virtio-serial tries to
> copy the page contents to an allocated page, instead
> of just failing splice().
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Amit Shah <amit.shah@redhat.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>
> drivers/char/virtio_console.c | 28 +++++++++++++++++++++++++---
> 1 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index fe31b2f..911cb3e 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -794,7 +794,7 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
> struct splice_desc *sd)
> {
> struct sg_list *sgl = sd->u.data;
> - unsigned int len = 0;
> + unsigned int offset, len;
>
> if (sgl->n == MAX_SPLICE_PAGES)
> return 0;
> @@ -807,9 +807,31 @@ static int pipe_to_sg(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
>
> len = min(buf->len, sd->len);
> sg_set_page(&(sgl->sg[sgl->n]), buf->page, len, buf->offset);
> - sgl->n++;
> - sgl->len += len;
> + } else {
> + /* Failback to copying a page */
> + struct page *page = alloc_page(GFP_KERNEL);
I prefer zeroing out the page. If there's not enough data to be
filled in the page, the remaining data can be leaked to the host.
Amit
^ permalink raw reply
* Re: [PATCH v6 1/3] mm: introduce compaction and migration for virtio ballooned pages
From: Mel Gorman @ 2012-08-09 9:00 UTC (permalink / raw)
To: Rafael Aquini
Cc: Rik van Riel, Konrad Rzeszutek Wilk, Michael S. Tsirkin,
linux-kernel, virtualization, linux-mm, Andi Kleen, Minchan Kim,
Andrew Morton
In-Reply-To: <efb9756c5d6de8952a793bfc99a9db9cdd66b12f.1344463786.git.aquini@redhat.com>
On Wed, Aug 08, 2012 at 07:53:19PM -0300, Rafael Aquini wrote:
> Memory fragmentation introduced by ballooning might reduce significantly
> the number of 2MB contiguous memory blocks that can be used within a guest,
> thus imposing performance penalties associated with the reduced number of
> transparent huge pages that could be used by the guest workload.
>
> This patch introduces the helper functions as well as the necessary changes
> to teach compaction and migration bits how to cope with pages which are
> part of a guest memory balloon, in order to make them movable by memory
> compaction procedures.
>
> Signed-off-by: Rafael Aquini <aquini@redhat.com>
Mostly looks ok but I have one question;
> <SNIP>
>
> +/* putback_lru_page() counterpart for a ballooned page */
> +bool putback_balloon_page(struct page *page)
> +{
> + if (WARN_ON(!movable_balloon_page(page)))
> + return false;
> +
> + if (likely(trylock_page(page))) {
> + if (movable_balloon_page(page)) {
> + __putback_balloon_page(page);
> + put_page(page);
> + unlock_page(page);
> + return true;
> + }
> + unlock_page(page);
> + }
You might have answered this already as I skipped over a few revisions
and if you have, sorry about that and please add a comment :)
This trylock_page looks risky as it looks like it can fail if another
process running compaction tries to isolate this page. It locks the page,
finds it cant and releases the lock but in the meantime this trylock can
fail. It triggers a WARN_ON so we'll get a bug report but it leaves the
reference count elevated and this page has now leaked.
Why not just lock_page(page)? As you have already isolated this page you
know that the lock is only going to be held by a parallel compacting
process checking the reference count and the delay will be short. As a
bonus you can drop the WARN_ON check in the caller and make this void as
the WARN_ON check in the caller becomes redundant.
--
Mel Gorman
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH 1/6] virtio/console: Add splice_write support
From: Amit Shah @ 2012-08-09 9:00 UTC (permalink / raw)
To: Yoshihiro YUNOMAE
Cc: Arnd Bergmann, qemu-devel, Frederic Weisbecker, yrl.pp-manager.tt,
linux-kernel, Borislav Petkov, virtualization, Herbert Xu,
Franch Ch. Eigler, Ingo Molnar, Mathieu Desnoyers, Steven Rostedt,
Anthony Liguori, Greg Kroah-Hartman, Masami Hiramatsu
In-Reply-To: <20120724023707.6600.69536.stgit@ltc189.sdl.hitachi.co.jp>
On (Tue) 24 Jul 2012 [11:37:07], Yoshihiro YUNOMAE wrote:
> From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>
> Enable to use splice_write from pipe to virtio-console port.
> This steals pages from pipe and directly send it to host.
>
> Note that this may accelerate only the guest to host path.
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Cc: Amit Shah <amit.shah@redhat.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> +/* Faster zero-copy write by splicing */
> +static ssize_t port_fops_splice_write(struct pipe_inode_info *pipe,
> + struct file *filp, loff_t *ppos,
> + size_t len, unsigned int flags)
> +{
> + struct port *port = filp->private_data;
> + struct sg_list sgl;
> + ssize_t ret;
> + struct splice_desc sd = {
> + .total_len = len,
> + .flags = flags,
> + .pos = *ppos,
> + .u.data = &sgl,
> + };
> +
> + sgl.n = 0;
> + sgl.len = 0;
> + sgl.sg = kmalloc(sizeof(struct scatterlist) * MAX_SPLICE_PAGES,
> + GFP_ATOMIC);
Do you expect this function to be called from interrupt context?
> + if (unlikely(!sgl.sg))
> + return -ENOMEM;
> +
> + sg_init_table(sgl.sg, MAX_SPLICE_PAGES);
> + ret = __splice_from_pipe(pipe, &sd, pipe_to_sg);
> + if (likely(ret > 0))
> + ret = send_pages(port, sgl.sg, sgl.n, sgl.len, true);
> +
> + return ret;
> +}
Amit
^ permalink raw reply
* Re: [PATCH v6 1/3] mm: introduce compaction and migration for virtio ballooned pages
From: Minchan Kim @ 2012-08-09 1:55 UTC (permalink / raw)
To: Rafael Aquini
Cc: Rik van Riel, Konrad Rzeszutek Wilk, Michael S. Tsirkin,
linux-kernel, virtualization, linux-mm, Andi Kleen, Andrew Morton
In-Reply-To: <efb9756c5d6de8952a793bfc99a9db9cdd66b12f.1344463786.git.aquini@redhat.com>
Hi Rafael,
On Wed, Aug 08, 2012 at 07:53:19PM -0300, Rafael Aquini wrote:
> Memory fragmentation introduced by ballooning might reduce significantly
> the number of 2MB contiguous memory blocks that can be used within a guest,
> thus imposing performance penalties associated with the reduced number of
> transparent huge pages that could be used by the guest workload.
>
> This patch introduces the helper functions as well as the necessary changes
> to teach compaction and migration bits how to cope with pages which are
> part of a guest memory balloon, in order to make them movable by memory
> compaction procedures.
>
> Signed-off-by: Rafael Aquini <aquini@redhat.com>
> ---
> include/linux/mm.h | 17 +++++++
> mm/compaction.c | 131 +++++++++++++++++++++++++++++++++++++++++++++--------
> mm/migrate.c | 30 +++++++++++-
> 3 files changed, 158 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 311be90..18f978b 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1662,5 +1662,22 @@ static inline unsigned int debug_guardpage_minorder(void) { return 0; }
> static inline bool page_is_guard(struct page *page) { return false; }
> #endif /* CONFIG_DEBUG_PAGEALLOC */
>
> +#if (defined(CONFIG_VIRTIO_BALLOON) || \
> + defined(CONFIG_VIRTIO_BALLOON_MODULE)) && defined(CONFIG_COMPACTION)
> +extern bool isolate_balloon_page(struct page *);
> +extern bool putback_balloon_page(struct page *);
> +extern struct address_space *balloon_mapping;
> +
> +static inline bool movable_balloon_page(struct page *page)
> +{
> + return (page->mapping && page->mapping == balloon_mapping);
> +}
> +
> +#else
> +static inline bool isolate_balloon_page(struct page *page) { return false; }
> +static inline bool putback_balloon_page(struct page *page) { return false; }
> +static inline bool movable_balloon_page(struct page *page) { return false; }
> +#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && CONFIG_COMPACTION */
> +
> #endif /* __KERNEL__ */
> #endif /* _LINUX_MM_H */
> diff --git a/mm/compaction.c b/mm/compaction.c
> index e78cb96..7372592 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -14,6 +14,7 @@
> #include <linux/backing-dev.h>
> #include <linux/sysctl.h>
> #include <linux/sysfs.h>
> +#include <linux/export.h>
> #include "internal.h"
>
> #if defined CONFIG_COMPACTION || defined CONFIG_CMA
> @@ -21,6 +22,90 @@
> #define CREATE_TRACE_POINTS
> #include <trace/events/compaction.h>
>
> +#if defined(CONFIG_VIRTIO_BALLOON) || defined(CONFIG_VIRTIO_BALLOON_MODULE)
> +/*
> + * Balloon pages special page->mapping.
> + * Users must properly allocate and initialize an instance of balloon_mapping,
> + * and set it as the page->mapping for balloon enlisted page instances.
> + * There is no need on utilizing struct address_space locking schemes for
> + * balloon_mapping as, once it gets initialized at balloon driver, it will
> + * remain just like a static reference that helps us on identifying a guest
> + * ballooned page by its mapping, as well as it will keep the 'a_ops' callback
> + * pointers to the functions that will execute the balloon page mobility tasks.
> + *
> + * address_space_operations necessary methods for ballooned pages:
> + * .migratepage - used to perform balloon's page migration (as is)
> + * .invalidatepage - used to isolate a page from balloon's page list
> + * .freepage - used to reinsert an isolated page to balloon's page list
> + */
> +struct address_space *balloon_mapping;
> +EXPORT_SYMBOL_GPL(balloon_mapping);
> +
> +static inline void __isolate_balloon_page(struct page *page)
> +{
> + page->mapping->a_ops->invalidatepage(page, 0);
> +}
> +
> +static inline void __putback_balloon_page(struct page *page)
> +{
> + page->mapping->a_ops->freepage(page);
> +}
> +
> +/* __isolate_lru_page() counterpart for a ballooned page */
> +bool isolate_balloon_page(struct page *page)
> +{
> + if (WARN_ON(!movable_balloon_page(page)))
> + return false;
> +
> + if (likely(get_page_unless_zero(page))) {
> + /*
> + * As balloon pages are not isolated from LRU lists, concurrent
> + * compaction threads can race against page migration functions
> + * move_to_new_page() & __unmap_and_move().
> + * In order to avoid having an already isolated balloon page
> + * being (wrongly) re-isolated while it is under migration,
> + * lets be sure we have the page lock before proceeding with
> + * the balloon page isolation steps.
> + */
> + if (likely(trylock_page(page))) {
> + /*
> + * A ballooned page, by default, has just one refcount.
> + * Prevent concurrent compaction threads from isolating
> + * an already isolated balloon page.
> + */
> + if (movable_balloon_page(page) &&
> + (page_count(page) == 2)) {
> + __isolate_balloon_page(page);
> + unlock_page(page);
> + return true;
> + }
> + unlock_page(page);
> + }
> + /* Drop refcount taken for this already isolated page */
> + put_page(page);
> + }
> + return false;
> +}
> +
> +/* putback_lru_page() counterpart for a ballooned page */
> +bool putback_balloon_page(struct page *page)
> +{
> + if (WARN_ON(!movable_balloon_page(page)))
> + return false;
> +
> + if (likely(trylock_page(page))) {
> + if (movable_balloon_page(page)) {
> + __putback_balloon_page(page);
> + put_page(page);
> + unlock_page(page);
> + return true;
> + }
> + unlock_page(page);
> + }
> + return false;
> +}
> +#endif /* CONFIG_VIRTIO_BALLOON || CONFIG_VIRTIO_BALLOON_MODULE */
> +
> static unsigned long release_freepages(struct list_head *freelist)
> {
> struct page *page, *next;
> @@ -312,32 +397,40 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
> continue;
> }
>
> - if (!PageLRU(page))
> - continue;
> -
> /*
> - * PageLRU is set, and lru_lock excludes isolation,
> - * splitting and collapsing (collapsing has already
> - * happened if PageLRU is set).
> + * It is possible to migrate LRU pages and balloon pages.
> + * Skip any other type of page.
> */
> - if (PageTransHuge(page)) {
> - low_pfn += (1 << compound_order(page)) - 1;
> - continue;
> - }
> + if (PageLRU(page)) {
> + /*
> + * PageLRU is set, and lru_lock excludes isolation,
> + * splitting and collapsing (collapsing has already
> + * happened if PageLRU is set).
> + */
> + if (PageTransHuge(page)) {
> + low_pfn += (1 << compound_order(page)) - 1;
> + continue;
> + }
>
> - if (!cc->sync)
> - mode |= ISOLATE_ASYNC_MIGRATE;
> + if (!cc->sync)
> + mode |= ISOLATE_ASYNC_MIGRATE;
>
> - lruvec = mem_cgroup_page_lruvec(page, zone);
> + lruvec = mem_cgroup_page_lruvec(page, zone);
>
> - /* Try isolate the page */
> - if (__isolate_lru_page(page, mode) != 0)
> - continue;
> + /* Try isolate the page */
> + if (__isolate_lru_page(page, mode) != 0)
> + continue;
> +
> + VM_BUG_ON(PageTransCompound(page));
>
> - VM_BUG_ON(PageTransCompound(page));
> + /* Successfully isolated */
> + del_page_from_lru_list(page, lruvec, page_lru(page));
> + } else if (unlikely(movable_balloon_page(page))) {
> + if (!isolate_balloon_page(page))
> + continue;
> + } else
> + continue;
>
> - /* Successfully isolated */
> - del_page_from_lru_list(page, lruvec, page_lru(page));
> list_add(&page->lru, migratelist);
> cc->nr_migratepages++;
> nr_isolated++;
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 77ed2d7..871a304 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -79,7 +79,10 @@ void putback_lru_pages(struct list_head *l)
> list_del(&page->lru);
> dec_zone_page_state(page, NR_ISOLATED_ANON +
> page_is_file_cache(page));
> - putback_lru_page(page);
> + if (unlikely(movable_balloon_page(page)))
> + WARN_ON(!putback_balloon_page(page));
> + else
> + putback_lru_page(page);
> }
Don't hack putback_lru_pages. It's a function for handling LRU pages
and is used by several places.
Plz, don't add complexity to unrelavant parts.
You can define a new function putback_migratepages should be used as pair with
isolate_migratepages_range so that both functions are aware of balloon page.
IMHO, it's better abstraction rather than hook of generic function.
Otherwise, Looks good to me.
Thanks.
--
Kind regards,
Minchan Kim
^ permalink raw reply
* Re: [PATCH v6 3/3] mm: add vm event counters for balloon pages compaction
From: Rik van Riel @ 2012-08-09 1:40 UTC (permalink / raw)
To: Rafael Aquini
Cc: Konrad Rzeszutek Wilk, Michael S. Tsirkin, linux-kernel,
virtualization, linux-mm, Andi Kleen, Minchan Kim, Andrew Morton
In-Reply-To: <768e09520a249fd78f706cd8ae53d511f9db0aaa.1344463786.git.aquini@redhat.com>
On 08/08/2012 06:53 PM, Rafael Aquini wrote:
> This patch is only for testing report purposes and shall be dropped in case of
> the rest of this patchset getting accepted for merging.
I wonder if it would make sense to just keep these statistics, so
if a change breaks balloon migration in the future, we'll be able
to see it...
> Signed-off-by: Rafael Aquini<aquini@redhat.com>
Acked-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
^ permalink raw reply
* Re: [PATCH v6 2/3] virtio_balloon: introduce migration primitives to balloon pages
From: Rik van Riel @ 2012-08-09 1:39 UTC (permalink / raw)
To: Rafael Aquini
Cc: Konrad Rzeszutek Wilk, Michael S. Tsirkin, linux-kernel,
virtualization, linux-mm, Andi Kleen, Minchan Kim, Andrew Morton
In-Reply-To: <5f4b30060e252e557882595984a8225ba6e0c9f2.1344463786.git.aquini@redhat.com>
On 08/08/2012 06:53 PM, Rafael Aquini wrote:
> Memory fragmentation introduced by ballooning might reduce significantly
> the number of 2MB contiguous memory blocks that can be used within a guest,
> thus imposing performance penalties associated with the reduced number of
> transparent huge pages that could be used by the guest workload.
>
> Besides making balloon pages movable at allocation time and introducing
> the necessary primitives to perform balloon page migration/compaction,
> this patch also introduces the following locking scheme to provide the
> proper synchronization and protection for struct virtio_balloon elements
> against concurrent accesses due to parallel operations introduced by
> memory compaction / page migration.
> - balloon_lock (mutex) : synchronizes the access demand to elements of
> struct virtio_balloon and its queue operations;
> - pages_lock (spinlock): special protection to balloon pages list against
> concurrent list handling operations;
>
> Signed-off-by: Rafael Aquini<aquini@redhat.com>
Reviewed-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
^ permalink raw reply
* Re: [PATCH v6 1/3] mm: introduce compaction and migration for virtio ballooned pages
From: Rik van Riel @ 2012-08-09 1:37 UTC (permalink / raw)
To: Rafael Aquini
Cc: Konrad Rzeszutek Wilk, Michael S. Tsirkin, linux-kernel,
virtualization, linux-mm, Andi Kleen, Minchan Kim, Andrew Morton
In-Reply-To: <efb9756c5d6de8952a793bfc99a9db9cdd66b12f.1344463786.git.aquini@redhat.com>
On 08/08/2012 06:53 PM, Rafael Aquini wrote:
> Memory fragmentation introduced by ballooning might reduce significantly
> the number of 2MB contiguous memory blocks that can be used within a guest,
> thus imposing performance penalties associated with the reduced number of
> transparent huge pages that could be used by the guest workload.
>
> This patch introduces the helper functions as well as the necessary changes
> to teach compaction and migration bits how to cope with pages which are
> part of a guest memory balloon, in order to make them movable by memory
> compaction procedures.
>
> Signed-off-by: Rafael Aquini<aquini@redhat.com>
Reviewed-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
^ permalink raw reply
* [PATCH v6 3/3] mm: add vm event counters for balloon pages compaction
From: Rafael Aquini @ 2012-08-08 22:53 UTC (permalink / raw)
To: linux-mm
Cc: Rik van Riel, Rafael Aquini, Konrad Rzeszutek Wilk,
Michael S. Tsirkin, linux-kernel, virtualization, Minchan Kim,
Andi Kleen, Andrew Morton
In-Reply-To: <cover.1344463786.git.aquini@redhat.com>
This patch is only for testing report purposes and shall be dropped in case of
the rest of this patchset getting accepted for merging.
Signed-off-by: Rafael Aquini <aquini@redhat.com>
---
drivers/virtio/virtio_balloon.c | 1 +
include/linux/vm_event_item.h | 2 ++
mm/compaction.c | 1 +
mm/migrate.c | 6 ++++--
mm/vmstat.c | 4 ++++
5 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7c937a0..b8f7ea5 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -414,6 +414,7 @@ int virtballoon_migratepage(struct address_space *mapping,
mutex_unlock(&balloon_lock);
+ count_vm_event(COMPACTBALLOONMIGRATED);
return 0;
}
diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
index 57f7b10..a632a5d 100644
--- a/include/linux/vm_event_item.h
+++ b/include/linux/vm_event_item.h
@@ -41,6 +41,8 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
#ifdef CONFIG_COMPACTION
COMPACTBLOCKS, COMPACTPAGES, COMPACTPAGEFAILED,
COMPACTSTALL, COMPACTFAIL, COMPACTSUCCESS,
+ COMPACTBALLOONMIGRATED, COMPACTBALLOONFAILED,
+ COMPACTBALLOONISOLATED, COMPACTBALLOONFREED,
#endif
#ifdef CONFIG_HUGETLB_PAGE
HTLB_BUDDY_PGALLOC, HTLB_BUDDY_PGALLOC_FAIL,
diff --git a/mm/compaction.c b/mm/compaction.c
index 7372592..5d6a344 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -77,6 +77,7 @@ bool isolate_balloon_page(struct page *page)
(page_count(page) == 2)) {
__isolate_balloon_page(page);
unlock_page(page);
+ count_vm_event(COMPACTBALLOONISOLATED);
return true;
}
unlock_page(page);
diff --git a/mm/migrate.c b/mm/migrate.c
index 871a304..4115875 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -79,9 +79,10 @@ void putback_lru_pages(struct list_head *l)
list_del(&page->lru);
dec_zone_page_state(page, NR_ISOLATED_ANON +
page_is_file_cache(page));
- if (unlikely(movable_balloon_page(page)))
+ if (unlikely(movable_balloon_page(page))) {
+ count_vm_event(COMPACTBALLOONFAILED);
WARN_ON(!putback_balloon_page(page));
- else
+ } else
putback_lru_page(page);
}
}
@@ -872,6 +873,7 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
page_is_file_cache(page));
put_page(page);
__free_page(page);
+ count_vm_event(COMPACTBALLOONFREED);
return rc;
}
out:
diff --git a/mm/vmstat.c b/mm/vmstat.c
index df7a674..8d80f60 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -768,6 +768,10 @@ const char * const vmstat_text[] = {
"compact_stall",
"compact_fail",
"compact_success",
+ "compact_balloon_migrated",
+ "compact_balloon_failed",
+ "compact_balloon_isolated",
+ "compact_balloon_freed",
#endif
#ifdef CONFIG_HUGETLB_PAGE
--
1.7.11.2
^ permalink raw reply related
* [PATCH v6 2/3] virtio_balloon: introduce migration primitives to balloon pages
From: Rafael Aquini @ 2012-08-08 22:53 UTC (permalink / raw)
To: linux-mm
Cc: Rik van Riel, Rafael Aquini, Konrad Rzeszutek Wilk,
Michael S. Tsirkin, linux-kernel, virtualization, Minchan Kim,
Andi Kleen, Andrew Morton
In-Reply-To: <cover.1344463786.git.aquini@redhat.com>
Memory fragmentation introduced by ballooning might reduce significantly
the number of 2MB contiguous memory blocks that can be used within a guest,
thus imposing performance penalties associated with the reduced number of
transparent huge pages that could be used by the guest workload.
Besides making balloon pages movable at allocation time and introducing
the necessary primitives to perform balloon page migration/compaction,
this patch also introduces the following locking scheme to provide the
proper synchronization and protection for struct virtio_balloon elements
against concurrent accesses due to parallel operations introduced by
memory compaction / page migration.
- balloon_lock (mutex) : synchronizes the access demand to elements of
struct virtio_balloon and its queue operations;
- pages_lock (spinlock): special protection to balloon pages list against
concurrent list handling operations;
Signed-off-by: Rafael Aquini <aquini@redhat.com>
---
drivers/virtio/virtio_balloon.c | 138 +++++++++++++++++++++++++++++++++++++---
include/linux/virtio_balloon.h | 4 ++
2 files changed, 134 insertions(+), 8 deletions(-)
diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0908e60..7c937a0 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -27,6 +27,7 @@
#include <linux/delay.h>
#include <linux/slab.h>
#include <linux/module.h>
+#include <linux/fs.h>
/*
* Balloon device works in 4K page units. So each page is pointed to by
@@ -35,6 +36,12 @@
*/
#define VIRTIO_BALLOON_PAGES_PER_PAGE (PAGE_SIZE >> VIRTIO_BALLOON_PFN_SHIFT)
+/* Synchronizes accesses/updates to the struct virtio_balloon elements */
+DEFINE_MUTEX(balloon_lock);
+
+/* Protects 'virtio_balloon->pages' list against concurrent handling */
+DEFINE_SPINLOCK(pages_lock);
+
struct virtio_balloon
{
struct virtio_device *vdev;
@@ -51,6 +58,7 @@ struct virtio_balloon
/* Number of balloon pages we've told the Host we're not using. */
unsigned int num_pages;
+
/*
* The pages we've told the Host we're not using.
* Each page on this list adds VIRTIO_BALLOON_PAGES_PER_PAGE
@@ -125,10 +133,12 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb->pfns));
+ mutex_lock(&balloon_lock);
for (vb->num_pfns = 0; vb->num_pfns < num;
vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
- struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY |
- __GFP_NOMEMALLOC | __GFP_NOWARN);
+ struct page *page = alloc_page(GFP_HIGHUSER_MOVABLE |
+ __GFP_NORETRY | __GFP_NOWARN |
+ __GFP_NOMEMALLOC);
if (!page) {
if (printk_ratelimit())
dev_printk(KERN_INFO, &vb->vdev->dev,
@@ -141,7 +151,10 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
set_page_pfns(vb->pfns + vb->num_pfns, page);
vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
totalram_pages--;
+ spin_lock(&pages_lock);
list_add(&page->lru, &vb->pages);
+ page->mapping = balloon_mapping;
+ spin_unlock(&pages_lock);
}
/* Didn't get any? Oh well. */
@@ -149,6 +162,7 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
return;
tell_host(vb, vb->inflate_vq);
+ mutex_unlock(&balloon_lock);
}
static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
@@ -169,10 +183,22 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb->pfns));
+ mutex_lock(&balloon_lock);
for (vb->num_pfns = 0; vb->num_pfns < num;
vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
+ /*
+ * We can race against virtballoon_isolatepage() and end up
+ * stumbling across a _temporarily_ empty 'pages' list.
+ */
+ spin_lock(&pages_lock);
+ if (unlikely(list_empty(&vb->pages))) {
+ spin_unlock(&pages_lock);
+ break;
+ }
page = list_first_entry(&vb->pages, struct page, lru);
+ page->mapping = NULL;
list_del(&page->lru);
+ spin_unlock(&pages_lock);
set_page_pfns(vb->pfns + vb->num_pfns, page);
vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
}
@@ -182,8 +208,11 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
* virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
* is true, we *have* to do it in this order
*/
- tell_host(vb, vb->deflate_vq);
- release_pages_by_pfn(vb->pfns, vb->num_pfns);
+ if (vb->num_pfns > 0) {
+ tell_host(vb, vb->deflate_vq);
+ release_pages_by_pfn(vb->pfns, vb->num_pfns);
+ }
+ mutex_unlock(&balloon_lock);
}
static inline void update_stat(struct virtio_balloon *vb, int idx,
@@ -239,6 +268,7 @@ static void stats_handle_request(struct virtio_balloon *vb)
struct scatterlist sg;
unsigned int len;
+ mutex_lock(&balloon_lock);
vb->need_stats_update = 0;
update_balloon_stats(vb);
@@ -249,6 +279,7 @@ static void stats_handle_request(struct virtio_balloon *vb)
if (virtqueue_add_buf(vq, &sg, 1, 0, vb, GFP_KERNEL) < 0)
BUG();
virtqueue_kick(vq);
+ mutex_unlock(&balloon_lock);
}
static void virtballoon_changed(struct virtio_device *vdev)
@@ -261,22 +292,27 @@ static void virtballoon_changed(struct virtio_device *vdev)
static inline s64 towards_target(struct virtio_balloon *vb)
{
__le32 v;
- s64 target;
+ s64 target, actual;
+ mutex_lock(&balloon_lock);
+ actual = vb->num_pages;
vb->vdev->config->get(vb->vdev,
offsetof(struct virtio_balloon_config, num_pages),
&v, sizeof(v));
target = le32_to_cpu(v);
- return target - vb->num_pages;
+ mutex_unlock(&balloon_lock);
+ return target - actual;
}
static void update_balloon_size(struct virtio_balloon *vb)
{
- __le32 actual = cpu_to_le32(vb->num_pages);
-
+ __le32 actual;
+ mutex_lock(&balloon_lock);
+ actual = cpu_to_le32(vb->num_pages);
vb->vdev->config->set(vb->vdev,
offsetof(struct virtio_balloon_config, actual),
&actual, sizeof(actual));
+ mutex_unlock(&balloon_lock);
}
static int balloon(void *_vballoon)
@@ -339,6 +375,76 @@ static int init_vqs(struct virtio_balloon *vb)
return 0;
}
+/*
+ * '*vb_ptr' allows virtballoon_migratepage() & virtballoon_putbackpage() to
+ * access pertinent elements from struct virtio_balloon
+ */
+struct virtio_balloon *vb_ptr;
+
+/*
+ * Populate balloon_mapping->a_ops->migratepage method to perform the balloon
+ * page migration task.
+ *
+ * After a ballooned page gets isolated by compaction procedures, this is the
+ * function that performs the page migration on behalf of move_to_new_page(),
+ * when the last calls (page)->mapping->a_ops->migratepage.
+ *
+ * Page migration for virtio balloon is done in a simple swap fashion which
+ * follows these two steps:
+ * 1) insert newpage into vb->pages list and update the host about it;
+ * 2) update the host about the removed old page from vb->pages list;
+ */
+int virtballoon_migratepage(struct address_space *mapping,
+ struct page *newpage, struct page *page, enum migrate_mode mode)
+{
+ mutex_lock(&balloon_lock);
+
+ /* balloon's page migration 1st step */
+ vb_ptr->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
+ spin_lock(&pages_lock);
+ list_add(&newpage->lru, &vb_ptr->pages);
+ spin_unlock(&pages_lock);
+ set_page_pfns(vb_ptr->pfns, newpage);
+ tell_host(vb_ptr, vb_ptr->inflate_vq);
+
+ /* balloon's page migration 2nd step */
+ vb_ptr->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
+ set_page_pfns(vb_ptr->pfns, page);
+ tell_host(vb_ptr, vb_ptr->deflate_vq);
+
+ mutex_unlock(&balloon_lock);
+
+ return 0;
+}
+
+/*
+ * Populate balloon_mapping->a_ops->invalidatepage method to help compaction on
+ * isolating a page from the balloon page list.
+ */
+void virtballoon_isolatepage(struct page *page, unsigned long mode)
+{
+ spin_lock(&pages_lock);
+ list_del(&page->lru);
+ spin_unlock(&pages_lock);
+}
+
+/*
+ * Populate balloon_mapping->a_ops->freepage method to help compaction on
+ * re-inserting an isolated page into the balloon page list.
+ */
+void virtballoon_putbackpage(struct page *page)
+{
+ spin_lock(&pages_lock);
+ list_add(&page->lru, &vb_ptr->pages);
+ spin_unlock(&pages_lock);
+}
+
+static const struct address_space_operations virtio_balloon_aops = {
+ .migratepage = virtballoon_migratepage,
+ .invalidatepage = virtballoon_isolatepage,
+ .freepage = virtballoon_putbackpage,
+};
+
static int virtballoon_probe(struct virtio_device *vdev)
{
struct virtio_balloon *vb;
@@ -351,11 +457,25 @@ static int virtballoon_probe(struct virtio_device *vdev)
}
INIT_LIST_HEAD(&vb->pages);
+
vb->num_pages = 0;
init_waitqueue_head(&vb->config_change);
init_waitqueue_head(&vb->acked);
vb->vdev = vdev;
vb->need_stats_update = 0;
+ vb_ptr = vb;
+
+ /* Init the ballooned page->mapping special balloon_mapping */
+ balloon_mapping = kmalloc(sizeof(*balloon_mapping), GFP_KERNEL);
+ if (!balloon_mapping) {
+ err = -ENOMEM;
+ goto out_free_vb;
+ }
+
+ INIT_RADIX_TREE(&balloon_mapping->page_tree, GFP_ATOMIC | __GFP_NOWARN);
+ INIT_LIST_HEAD(&balloon_mapping->i_mmap_nonlinear);
+ spin_lock_init(&balloon_mapping->tree_lock);
+ balloon_mapping->a_ops = &virtio_balloon_aops;
err = init_vqs(vb);
if (err)
@@ -373,6 +493,7 @@ out_del_vqs:
vdev->config->del_vqs(vdev);
out_free_vb:
kfree(vb);
+ kfree(balloon_mapping);
out:
return err;
}
@@ -397,6 +518,7 @@ static void __devexit virtballoon_remove(struct virtio_device *vdev)
kthread_stop(vb->thread);
remove_common(vb);
kfree(vb);
+ kfree(balloon_mapping);
}
#ifdef CONFIG_PM
diff --git a/include/linux/virtio_balloon.h b/include/linux/virtio_balloon.h
index 652dc8b..930f1b7 100644
--- a/include/linux/virtio_balloon.h
+++ b/include/linux/virtio_balloon.h
@@ -56,4 +56,8 @@ struct virtio_balloon_stat {
u64 val;
} __attribute__((packed));
+#if !defined(CONFIG_COMPACTION)
+struct address_space *balloon_mapping;
+#endif
+
#endif /* _LINUX_VIRTIO_BALLOON_H */
--
1.7.11.2
^ permalink raw reply related
* [PATCH v6 1/3] mm: introduce compaction and migration for virtio ballooned pages
From: Rafael Aquini @ 2012-08-08 22:53 UTC (permalink / raw)
To: linux-mm
Cc: Rik van Riel, Rafael Aquini, Konrad Rzeszutek Wilk,
Michael S. Tsirkin, linux-kernel, virtualization, Minchan Kim,
Andi Kleen, Andrew Morton
In-Reply-To: <cover.1344463786.git.aquini@redhat.com>
Memory fragmentation introduced by ballooning might reduce significantly
the number of 2MB contiguous memory blocks that can be used within a guest,
thus imposing performance penalties associated with the reduced number of
transparent huge pages that could be used by the guest workload.
This patch introduces the helper functions as well as the necessary changes
to teach compaction and migration bits how to cope with pages which are
part of a guest memory balloon, in order to make them movable by memory
compaction procedures.
Signed-off-by: Rafael Aquini <aquini@redhat.com>
---
include/linux/mm.h | 17 +++++++
mm/compaction.c | 131 +++++++++++++++++++++++++++++++++++++++++++++--------
mm/migrate.c | 30 +++++++++++-
3 files changed, 158 insertions(+), 20 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 311be90..18f978b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1662,5 +1662,22 @@ static inline unsigned int debug_guardpage_minorder(void) { return 0; }
static inline bool page_is_guard(struct page *page) { return false; }
#endif /* CONFIG_DEBUG_PAGEALLOC */
+#if (defined(CONFIG_VIRTIO_BALLOON) || \
+ defined(CONFIG_VIRTIO_BALLOON_MODULE)) && defined(CONFIG_COMPACTION)
+extern bool isolate_balloon_page(struct page *);
+extern bool putback_balloon_page(struct page *);
+extern struct address_space *balloon_mapping;
+
+static inline bool movable_balloon_page(struct page *page)
+{
+ return (page->mapping && page->mapping == balloon_mapping);
+}
+
+#else
+static inline bool isolate_balloon_page(struct page *page) { return false; }
+static inline bool putback_balloon_page(struct page *page) { return false; }
+static inline bool movable_balloon_page(struct page *page) { return false; }
+#endif /* (VIRTIO_BALLOON || VIRTIO_BALLOON_MODULE) && CONFIG_COMPACTION */
+
#endif /* __KERNEL__ */
#endif /* _LINUX_MM_H */
diff --git a/mm/compaction.c b/mm/compaction.c
index e78cb96..7372592 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -14,6 +14,7 @@
#include <linux/backing-dev.h>
#include <linux/sysctl.h>
#include <linux/sysfs.h>
+#include <linux/export.h>
#include "internal.h"
#if defined CONFIG_COMPACTION || defined CONFIG_CMA
@@ -21,6 +22,90 @@
#define CREATE_TRACE_POINTS
#include <trace/events/compaction.h>
+#if defined(CONFIG_VIRTIO_BALLOON) || defined(CONFIG_VIRTIO_BALLOON_MODULE)
+/*
+ * Balloon pages special page->mapping.
+ * Users must properly allocate and initialize an instance of balloon_mapping,
+ * and set it as the page->mapping for balloon enlisted page instances.
+ * There is no need on utilizing struct address_space locking schemes for
+ * balloon_mapping as, once it gets initialized at balloon driver, it will
+ * remain just like a static reference that helps us on identifying a guest
+ * ballooned page by its mapping, as well as it will keep the 'a_ops' callback
+ * pointers to the functions that will execute the balloon page mobility tasks.
+ *
+ * address_space_operations necessary methods for ballooned pages:
+ * .migratepage - used to perform balloon's page migration (as is)
+ * .invalidatepage - used to isolate a page from balloon's page list
+ * .freepage - used to reinsert an isolated page to balloon's page list
+ */
+struct address_space *balloon_mapping;
+EXPORT_SYMBOL_GPL(balloon_mapping);
+
+static inline void __isolate_balloon_page(struct page *page)
+{
+ page->mapping->a_ops->invalidatepage(page, 0);
+}
+
+static inline void __putback_balloon_page(struct page *page)
+{
+ page->mapping->a_ops->freepage(page);
+}
+
+/* __isolate_lru_page() counterpart for a ballooned page */
+bool isolate_balloon_page(struct page *page)
+{
+ if (WARN_ON(!movable_balloon_page(page)))
+ return false;
+
+ if (likely(get_page_unless_zero(page))) {
+ /*
+ * As balloon pages are not isolated from LRU lists, concurrent
+ * compaction threads can race against page migration functions
+ * move_to_new_page() & __unmap_and_move().
+ * In order to avoid having an already isolated balloon page
+ * being (wrongly) re-isolated while it is under migration,
+ * lets be sure we have the page lock before proceeding with
+ * the balloon page isolation steps.
+ */
+ if (likely(trylock_page(page))) {
+ /*
+ * A ballooned page, by default, has just one refcount.
+ * Prevent concurrent compaction threads from isolating
+ * an already isolated balloon page.
+ */
+ if (movable_balloon_page(page) &&
+ (page_count(page) == 2)) {
+ __isolate_balloon_page(page);
+ unlock_page(page);
+ return true;
+ }
+ unlock_page(page);
+ }
+ /* Drop refcount taken for this already isolated page */
+ put_page(page);
+ }
+ return false;
+}
+
+/* putback_lru_page() counterpart for a ballooned page */
+bool putback_balloon_page(struct page *page)
+{
+ if (WARN_ON(!movable_balloon_page(page)))
+ return false;
+
+ if (likely(trylock_page(page))) {
+ if (movable_balloon_page(page)) {
+ __putback_balloon_page(page);
+ put_page(page);
+ unlock_page(page);
+ return true;
+ }
+ unlock_page(page);
+ }
+ return false;
+}
+#endif /* CONFIG_VIRTIO_BALLOON || CONFIG_VIRTIO_BALLOON_MODULE */
+
static unsigned long release_freepages(struct list_head *freelist)
{
struct page *page, *next;
@@ -312,32 +397,40 @@ isolate_migratepages_range(struct zone *zone, struct compact_control *cc,
continue;
}
- if (!PageLRU(page))
- continue;
-
/*
- * PageLRU is set, and lru_lock excludes isolation,
- * splitting and collapsing (collapsing has already
- * happened if PageLRU is set).
+ * It is possible to migrate LRU pages and balloon pages.
+ * Skip any other type of page.
*/
- if (PageTransHuge(page)) {
- low_pfn += (1 << compound_order(page)) - 1;
- continue;
- }
+ if (PageLRU(page)) {
+ /*
+ * PageLRU is set, and lru_lock excludes isolation,
+ * splitting and collapsing (collapsing has already
+ * happened if PageLRU is set).
+ */
+ if (PageTransHuge(page)) {
+ low_pfn += (1 << compound_order(page)) - 1;
+ continue;
+ }
- if (!cc->sync)
- mode |= ISOLATE_ASYNC_MIGRATE;
+ if (!cc->sync)
+ mode |= ISOLATE_ASYNC_MIGRATE;
- lruvec = mem_cgroup_page_lruvec(page, zone);
+ lruvec = mem_cgroup_page_lruvec(page, zone);
- /* Try isolate the page */
- if (__isolate_lru_page(page, mode) != 0)
- continue;
+ /* Try isolate the page */
+ if (__isolate_lru_page(page, mode) != 0)
+ continue;
+
+ VM_BUG_ON(PageTransCompound(page));
- VM_BUG_ON(PageTransCompound(page));
+ /* Successfully isolated */
+ del_page_from_lru_list(page, lruvec, page_lru(page));
+ } else if (unlikely(movable_balloon_page(page))) {
+ if (!isolate_balloon_page(page))
+ continue;
+ } else
+ continue;
- /* Successfully isolated */
- del_page_from_lru_list(page, lruvec, page_lru(page));
list_add(&page->lru, migratelist);
cc->nr_migratepages++;
nr_isolated++;
diff --git a/mm/migrate.c b/mm/migrate.c
index 77ed2d7..871a304 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -79,7 +79,10 @@ void putback_lru_pages(struct list_head *l)
list_del(&page->lru);
dec_zone_page_state(page, NR_ISOLATED_ANON +
page_is_file_cache(page));
- putback_lru_page(page);
+ if (unlikely(movable_balloon_page(page)))
+ WARN_ON(!putback_balloon_page(page));
+ else
+ putback_lru_page(page);
}
}
@@ -778,6 +781,17 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
}
}
+ if (unlikely(movable_balloon_page(page))) {
+ /*
+ * A ballooned page does not need any special attention from
+ * physical to virtual reverse mapping procedures.
+ * Skip any attempt to unmap PTEs or to remap swap cache,
+ * in order to avoid burning cycles at rmap level.
+ */
+ remap_swapcache = 0;
+ goto skip_unmap;
+ }
+
/*
* Corner case handling:
* 1. When a new swap-cache page is read into, it is added to the LRU
@@ -846,6 +860,20 @@ static int unmap_and_move(new_page_t get_new_page, unsigned long private,
goto out;
rc = __unmap_and_move(page, newpage, force, offlining, mode);
+
+ if (unlikely(movable_balloon_page(newpage))) {
+ /*
+ * A ballooned page has been migrated already. Now, it is the
+ * time to wrap-up counters, handle the old page back to Buddy
+ * and return.
+ */
+ list_del(&page->lru);
+ dec_zone_page_state(page, NR_ISOLATED_ANON +
+ page_is_file_cache(page));
+ put_page(page);
+ __free_page(page);
+ return rc;
+ }
out:
if (rc != -EAGAIN) {
/*
--
1.7.11.2
^ permalink raw reply related
* [PATCH v6 0/3] make balloon pages movable by compaction
From: Rafael Aquini @ 2012-08-08 22:53 UTC (permalink / raw)
To: linux-mm
Cc: Rik van Riel, Rafael Aquini, Konrad Rzeszutek Wilk,
Michael S. Tsirkin, linux-kernel, virtualization, Minchan Kim,
Andi Kleen, Andrew Morton
Memory fragmentation introduced by ballooning might reduce significantly
the number of 2MB contiguous memory blocks that can be used within a guest,
thus imposing performance penalties associated with the reduced number of
transparent huge pages that could be used by the guest workload.
This patch-set follows the main idea discussed at 2012 LSFMMS session:
"Ballooning for transparent huge pages" -- http://lwn.net/Articles/490114/
to introduce the required changes to the virtio_balloon driver, as well as
the changes to the core compaction & migration bits, in order to make those
subsystems aware of ballooned pages and allow memory balloon pages become
movable within a guest, thus avoiding the aforementioned fragmentation issue
Rafael Aquini (3):
mm: introduce compaction and migration for virtio ballooned pages
virtio_balloon: introduce migration primitives to balloon pages
mm: add vm event counters for balloon pages compaction
drivers/virtio/virtio_balloon.c | 139 +++++++++++++++++++++++++++++++++++++---
include/linux/mm.h | 17 +++++
include/linux/virtio_balloon.h | 4 ++
include/linux/vm_event_item.h | 2 +
mm/compaction.c | 132 ++++++++++++++++++++++++++++++++------
mm/migrate.c | 32 ++++++++-
mm/vmstat.c | 4 ++
7 files changed, 302 insertions(+), 28 deletions(-)
Change log:
v6:
* rename 'is_balloon_page()' to 'movable_balloon_page()' (Rik);
v5:
* address Andrew Morton's review comments on the patch series;
* address a couple extra nitpick suggestions on PATCH 01 (Minchan);
v4:
* address Rusty Russel's review comments on PATCH 02;
* re-base virtio_balloon patch on 9c378abc5c0c6fc8e3acf5968924d274503819b3;
V3:
* address reviewers nitpick suggestions on PATCH 01 (Mel, Minchan);
V2:
* address Mel Gorman's review comments on PATCH 01;
Preliminary test results:
(2 VCPU 1024mB RAM KVM guest running 3.6.0_rc1+ -- after a reboot)
* 64mB balloon:
[root@localhost ~]# awk '/compact/ {print}' /proc/vmstat
compact_blocks_moved 0
compact_pages_moved 0
compact_pagemigrate_failed 0
compact_stall 0
compact_fail 0
compact_success 0
compact_balloon_migrated 0
compact_balloon_failed 0
compact_balloon_isolated 0
compact_balloon_freed 0
[root@localhost ~]#
[root@localhost ~]# for i in $(seq 1 6); do echo 1 > /proc/sys/vm/compact_memory & done &>/dev/null
[1] Done echo 1 > /proc/sys/vm/compact_memory
[2] Done echo 1 > /proc/sys/vm/compact_memory
[3] Done echo 1 > /proc/sys/vm/compact_memory
[4] Done echo 1 > /proc/sys/vm/compact_memory
[5]- Done echo 1 > /proc/sys/vm/compact_memory
[6]+ Done echo 1 > /proc/sys/vm/compact_memory
[root@localhost ~]#
[root@localhost ~]# awk '/compact/ {print}' /proc/vmstat
compact_blocks_moved 3520
compact_pages_moved 47548
compact_pagemigrate_failed 120
compact_stall 0
compact_fail 0
compact_success 0
compact_balloon_migrated 16378
compact_balloon_failed 0
compact_balloon_isolated 16378
compact_balloon_freed 16378
* 128mB balloon:
[root@localhost ~]# awk '/compact/ {print}' /proc/vmstat
compact_blocks_moved 0
compact_pages_moved 0
compact_pagemigrate_failed 0
compact_stall 0
compact_fail 0
compact_success 0
compact_balloon_migrated 0
compact_balloon_failed 0
compact_balloon_isolated 0
compact_balloon_freed 0
[root@localhost ~]#
[root@localhost ~]# for i in $(seq 1 6); do echo 1 > /proc/sys/vm/compact_memory & done &>/dev/null
[1] Done echo 1 > /proc/sys/vm/compact_memory
[2] Done echo 1 > /proc/sys/vm/compact_memory
[3] Done echo 1 > /proc/sys/vm/compact_memory
[4] Done echo 1 > /proc/sys/vm/compact_memory
[5]- Done echo 1 > /proc/sys/vm/compact_memory
[6]+ Done echo 1 > /proc/sys/vm/compact_memory
[root@localhost ~]#
[root@localhost ~]# awk '/compact/ {print}' /proc/vmstat
compact_blocks_moved 3356
compact_pages_moved 47099
compact_pagemigrate_failed 158
compact_stall 0
compact_fail 0
compact_success 0
compact_balloon_migrated 26275
compact_balloon_failed 42
compact_balloon_isolated 26317
compact_balloon_freed 26275
--
1.7.11.2
^ permalink raw reply
* [PATCH V7 2/2] virtio-blk: Add REQ_FLUSH and REQ_FUA support to bio path
From: Asias He @ 2012-08-08 8:07 UTC (permalink / raw)
To: linux-kernel
Cc: Jens Axboe, kvm, Michael S. Tsirkin, virtualization, Tejun Heo,
Shaohua Li, Christoph Hellwig
In-Reply-To: <1344413225-1843-1-git-send-email-asias@redhat.com>
We need to support both REQ_FLUSH and REQ_FUA for bio based path since
it does not get the sequencing of REQ_FUA into REQ_FLUSH that request
based drivers can request.
REQ_FLUSH is emulated by:
A) If the bio has no data to write:
1. Send VIRTIO_BLK_T_FLUSH to device,
2. In the flush I/O completion handler, finish the bio
B) If the bio has data to write:
1. Send VIRTIO_BLK_T_FLUSH to device
2. In the flush I/O completion handler, send the actual write data to device
3. In the write I/O completion handler, finish the bio
REQ_FUA is emulated by:
1. Send the actual write data to device
2. In the write I/O completion handler, send VIRTIO_BLK_T_FLUSH to device
3. In the flush I/O completion handler, finish the bio
Changes in v7:
- Using vbr->flags to trace request type
- Dropped unnecessary struct virtio_blk *vblk parameter
- Reuse struct virtblk_req in bio done function
Cahnges in v6:
- Reworked REQ_FLUSH and REQ_FUA emulatation order
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: Shaohua Li <shli@kernel.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Asias He <asias@redhat.com>
---
drivers/block/virtio_blk.c | 272 +++++++++++++++++++++++++++++++--------------
1 file changed, 188 insertions(+), 84 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 95cfeed..2edfb5c 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -58,10 +58,20 @@ struct virtblk_req
struct bio *bio;
struct virtio_blk_outhdr out_hdr;
struct virtio_scsi_inhdr in_hdr;
+ struct work_struct work;
+ struct virtio_blk *vblk;
+ int flags;
u8 status;
struct scatterlist sg[];
};
+enum {
+ VBLK_IS_FLUSH = 1,
+ VBLK_REQ_FLUSH = 2,
+ VBLK_REQ_DATA = 4,
+ VBLK_REQ_FUA = 8,
+};
+
static inline int virtblk_result(struct virtblk_req *vbr)
{
switch (vbr->status) {
@@ -74,9 +84,133 @@ static inline int virtblk_result(struct virtblk_req *vbr)
}
}
-static inline void virtblk_request_done(struct virtio_blk *vblk,
- struct virtblk_req *vbr)
+static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk,
+ gfp_t gfp_mask)
+{
+ struct virtblk_req *vbr;
+
+ vbr = mempool_alloc(vblk->pool, gfp_mask);
+ if (vbr && use_bio)
+ sg_init_table(vbr->sg, vblk->sg_elems);
+
+ vbr->vblk = vblk;
+
+ return vbr;
+}
+
+static void virtblk_add_buf_wait(struct virtio_blk *vblk,
+ struct virtblk_req *vbr,
+ unsigned long out,
+ unsigned long in)
+{
+ DEFINE_WAIT(wait);
+
+ for (;;) {
+ prepare_to_wait_exclusive(&vblk->queue_wait, &wait,
+ TASK_UNINTERRUPTIBLE);
+
+ spin_lock_irq(vblk->disk->queue->queue_lock);
+ if (virtqueue_add_buf(vblk->vq, vbr->sg, out, in, vbr,
+ GFP_ATOMIC) < 0) {
+ spin_unlock_irq(vblk->disk->queue->queue_lock);
+ io_schedule();
+ } else {
+ virtqueue_kick(vblk->vq);
+ spin_unlock_irq(vblk->disk->queue->queue_lock);
+ break;
+ }
+
+ }
+
+ finish_wait(&vblk->queue_wait, &wait);
+}
+
+static inline void virtblk_add_req(struct virtblk_req *vbr,
+ unsigned int out, unsigned int in)
+{
+ struct virtio_blk *vblk = vbr->vblk;
+
+ spin_lock_irq(vblk->disk->queue->queue_lock);
+ if (unlikely(virtqueue_add_buf(vblk->vq, vbr->sg, out, in, vbr,
+ GFP_ATOMIC) < 0)) {
+ spin_unlock_irq(vblk->disk->queue->queue_lock);
+ virtblk_add_buf_wait(vblk, vbr, out, in);
+ return;
+ }
+ virtqueue_kick(vblk->vq);
+ spin_unlock_irq(vblk->disk->queue->queue_lock);
+}
+
+static int virtblk_bio_send_flush(struct virtblk_req *vbr)
+{
+ unsigned int out = 0, in = 0;
+
+ vbr->flags |= VBLK_IS_FLUSH;
+ vbr->out_hdr.type = VIRTIO_BLK_T_FLUSH;
+ vbr->out_hdr.sector = 0;
+ vbr->out_hdr.ioprio = 0;
+ sg_set_buf(&vbr->sg[out++], &vbr->out_hdr, sizeof(vbr->out_hdr));
+ sg_set_buf(&vbr->sg[out + in++], &vbr->status, sizeof(vbr->status));
+
+ virtblk_add_req(vbr, out, in);
+
+ return 0;
+}
+
+static int virtblk_bio_send_data(struct virtblk_req *vbr)
{
+ struct virtio_blk *vblk = vbr->vblk;
+ unsigned int num, out = 0, in = 0;
+ struct bio *bio = vbr->bio;
+
+ vbr->flags &= ~VBLK_IS_FLUSH;
+ vbr->out_hdr.type = 0;
+ vbr->out_hdr.sector = bio->bi_sector;
+ vbr->out_hdr.ioprio = bio_prio(bio);
+
+ sg_set_buf(&vbr->sg[out++], &vbr->out_hdr, sizeof(vbr->out_hdr));
+
+ num = blk_bio_map_sg(vblk->disk->queue, bio, vbr->sg + out);
+
+ sg_set_buf(&vbr->sg[num + out + in++], &vbr->status,
+ sizeof(vbr->status));
+
+ if (num) {
+ if (bio->bi_rw & REQ_WRITE) {
+ vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
+ out += num;
+ } else {
+ vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
+ in += num;
+ }
+ }
+
+ virtblk_add_req(vbr, out, in);
+
+ return 0;
+}
+
+static void virtblk_bio_send_data_work(struct work_struct *work)
+{
+ struct virtblk_req *vbr;
+
+ vbr = container_of(work, struct virtblk_req, work);
+
+ virtblk_bio_send_data(vbr);
+}
+
+static void virtblk_bio_send_flush_work(struct work_struct *work)
+{
+ struct virtblk_req *vbr;
+
+ vbr = container_of(work, struct virtblk_req, work);
+
+ virtblk_bio_send_flush(vbr);
+}
+
+static inline void virtblk_request_done(struct virtblk_req *vbr)
+{
+ struct virtio_blk *vblk = vbr->vblk;
struct request *req = vbr->req;
int error = virtblk_result(vbr);
@@ -92,17 +226,47 @@ static inline void virtblk_request_done(struct virtio_blk *vblk,
mempool_free(vbr, vblk->pool);
}
-static inline void virtblk_bio_done(struct virtio_blk *vblk,
- struct virtblk_req *vbr)
+static inline void virtblk_bio_flush_done(struct virtblk_req *vbr)
{
- bio_endio(vbr->bio, virtblk_result(vbr));
- mempool_free(vbr, vblk->pool);
+ struct virtio_blk *vblk = vbr->vblk;
+
+ if (vbr->flags & VBLK_REQ_DATA) {
+ /* Send out the actual write data */
+ INIT_WORK(&vbr->work, virtblk_bio_send_data_work);
+ queue_work(virtblk_wq, &vbr->work);
+ } else {
+ bio_endio(vbr->bio, virtblk_result(vbr));
+ mempool_free(vbr, vblk->pool);
+ }
+}
+
+static inline void virtblk_bio_data_done(struct virtblk_req *vbr)
+{
+ struct virtio_blk *vblk = vbr->vblk;
+
+ if (unlikely(vbr->flags & VBLK_REQ_FUA)) {
+ /* Send out a flush before end the bio */
+ vbr->flags &= ~VBLK_REQ_DATA;
+ INIT_WORK(&vbr->work, virtblk_bio_send_flush_work);
+ queue_work(virtblk_wq, &vbr->work);
+ } else {
+ bio_endio(vbr->bio, virtblk_result(vbr));
+ mempool_free(vbr, vblk->pool);
+ }
+}
+
+static inline void virtblk_bio_done(struct virtblk_req *vbr)
+{
+ if (unlikely(vbr->flags & VBLK_IS_FLUSH))
+ virtblk_bio_flush_done(vbr);
+ else
+ virtblk_bio_data_done(vbr);
}
static void virtblk_done(struct virtqueue *vq)
{
struct virtio_blk *vblk = vq->vdev->priv;
- unsigned long bio_done = 0, req_done = 0;
+ bool bio_done = false, req_done = false;
struct virtblk_req *vbr;
unsigned long flags;
unsigned int len;
@@ -110,11 +274,11 @@ static void virtblk_done(struct virtqueue *vq)
spin_lock_irqsave(vblk->disk->queue->queue_lock, flags);
while ((vbr = virtqueue_get_buf(vblk->vq, &len)) != NULL) {
if (vbr->bio) {
- virtblk_bio_done(vblk, vbr);
- bio_done++;
+ virtblk_bio_done(vbr);
+ bio_done = true;
} else {
- virtblk_request_done(vblk, vbr);
- req_done++;
+ virtblk_request_done(vbr);
+ req_done = true;
}
}
/* In case queue is stopped waiting for more buffers. */
@@ -126,18 +290,6 @@ static void virtblk_done(struct virtqueue *vq)
wake_up(&vblk->queue_wait);
}
-static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk,
- gfp_t gfp_mask)
-{
- struct virtblk_req *vbr;
-
- vbr = mempool_alloc(vblk->pool, gfp_mask);
- if (vbr && use_bio)
- sg_init_table(vbr->sg, vblk->sg_elems);
-
- return vbr;
-}
-
static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
struct request *req)
{
@@ -242,41 +394,12 @@ static void virtblk_request(struct request_queue *q)
virtqueue_kick(vblk->vq);
}
-static void virtblk_add_buf_wait(struct virtio_blk *vblk,
- struct virtblk_req *vbr,
- unsigned long out,
- unsigned long in)
-{
- DEFINE_WAIT(wait);
-
- for (;;) {
- prepare_to_wait_exclusive(&vblk->queue_wait, &wait,
- TASK_UNINTERRUPTIBLE);
-
- spin_lock_irq(vblk->disk->queue->queue_lock);
- if (virtqueue_add_buf(vblk->vq, vbr->sg, out, in, vbr,
- GFP_ATOMIC) < 0) {
- spin_unlock_irq(vblk->disk->queue->queue_lock);
- io_schedule();
- } else {
- virtqueue_kick(vblk->vq);
- spin_unlock_irq(vblk->disk->queue->queue_lock);
- break;
- }
-
- }
-
- finish_wait(&vblk->queue_wait, &wait);
-}
-
static void virtblk_make_request(struct request_queue *q, struct bio *bio)
{
struct virtio_blk *vblk = q->queuedata;
- unsigned int num, out = 0, in = 0;
struct virtblk_req *vbr;
BUG_ON(bio->bi_phys_segments + 2 > vblk->sg_elems);
- BUG_ON(bio->bi_rw & (REQ_FLUSH | REQ_FUA));
vbr = virtblk_alloc_req(vblk, GFP_NOIO);
if (!vbr) {
@@ -285,37 +408,18 @@ static void virtblk_make_request(struct request_queue *q, struct bio *bio)
}
vbr->bio = bio;
- vbr->req = NULL;
- vbr->out_hdr.type = 0;
- vbr->out_hdr.sector = bio->bi_sector;
- vbr->out_hdr.ioprio = bio_prio(bio);
-
- sg_set_buf(&vbr->sg[out++], &vbr->out_hdr, sizeof(vbr->out_hdr));
-
- num = blk_bio_map_sg(q, bio, vbr->sg + out);
-
- sg_set_buf(&vbr->sg[num + out + in++], &vbr->status,
- sizeof(vbr->status));
-
- if (num) {
- if (bio->bi_rw & REQ_WRITE) {
- vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
- out += num;
- } else {
- vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
- in += num;
- }
- }
-
- spin_lock_irq(vblk->disk->queue->queue_lock);
- if (unlikely(virtqueue_add_buf(vblk->vq, vbr->sg, out, in, vbr,
- GFP_ATOMIC) < 0)) {
- spin_unlock_irq(vblk->disk->queue->queue_lock);
- virtblk_add_buf_wait(vblk, vbr, out, in);
- return;
- }
- virtqueue_kick(vblk->vq);
- spin_unlock_irq(vblk->disk->queue->queue_lock);
+ vbr->flags = 0;
+ if (bio->bi_rw & REQ_FLUSH)
+ vbr->flags |= VBLK_REQ_FLUSH;
+ if (bio->bi_rw & REQ_FUA)
+ vbr->flags |= VBLK_REQ_FUA;
+ if (bio->bi_size)
+ vbr->flags |= VBLK_REQ_DATA;
+
+ if (unlikely(vbr->flags & VBLK_REQ_FLUSH))
+ virtblk_bio_send_flush(vbr);
+ else
+ virtblk_bio_send_data(vbr);
}
/* return id (s/n) string for *disk to *id_str
@@ -529,7 +633,7 @@ static void virtblk_update_cache_mode(struct virtio_device *vdev)
u8 writeback = virtblk_get_cache_mode(vdev);
struct virtio_blk *vblk = vdev->priv;
- if (writeback && !use_bio)
+ if (writeback)
blk_queue_flush(vblk->disk->queue, REQ_FLUSH);
else
blk_queue_flush(vblk->disk->queue, 0);
--
1.7.11.2
^ permalink raw reply related
* [PATCH V7 1/2] virtio-blk: Add bio-based IO path for virtio-blk
From: Asias He @ 2012-08-08 8:07 UTC (permalink / raw)
To: linux-kernel
Cc: Jens Axboe, kvm, Michael S. Tsirkin, virtualization, Tejun Heo,
Shaohua Li, Christoph Hellwig
In-Reply-To: <1344413225-1843-1-git-send-email-asias@redhat.com>
This patch introduces bio-based IO path for virtio-blk.
Compared to request-based IO path, bio-based IO path uses driver
provided ->make_request_fn() method to bypasses the IO scheduler. It
handles the bio to device directly without allocating a request in block
layer. This reduces the IO path in guest kernel to achieve high IOPS
and lower latency. The downside is that guest can not use the IO
scheduler to merge and sort requests. However, this is not a big problem
if the backend disk in host side uses faster disk device.
When the bio-based IO path is not enabled, virtio-blk still uses the
original request-based IO path, no performance difference is observed.
Using a slow device e.g. normal SATA disk, the bio-based IO path for
sequential read and write are slower than req-based IO path due to lack
of merge in guest kernel. So we make the bio-based path optional.
Performance evaluation:
-----------------------------
1) Fio test is performed in a 8 vcpu guest with ramdisk based guest using
kvm tool.
Short version:
With bio-based IO path, sequential read/write, random read/write
IOPS boost : 28%, 24%, 21%, 16%
Latency improvement: 32%, 17%, 21%, 16%
Long version:
With bio-based IO path:
seq-read : io=2048.0MB, bw=116996KB/s, iops=233991 , runt= 17925msec
seq-write : io=2048.0MB, bw=100829KB/s, iops=201658 , runt= 20799msec
rand-read : io=3095.7MB, bw=112134KB/s, iops=224268 , runt= 28269msec
rand-write: io=3095.7MB, bw=96198KB/s, iops=192396 , runt= 32952msec
clat (usec): min=0 , max=2631.6K, avg=58716.99, stdev=191377.30
clat (usec): min=0 , max=1753.2K, avg=66423.25, stdev=81774.35
clat (usec): min=0 , max=2915.5K, avg=61685.70, stdev=120598.39
clat (usec): min=0 , max=1933.4K, avg=76935.12, stdev=96603.45
cpu : usr=74.08%, sys=703.84%, ctx=29661403, majf=21354, minf=22460954
cpu : usr=70.92%, sys=702.81%, ctx=77219828, majf=13980, minf=27713137
cpu : usr=72.23%, sys=695.37%, ctx=88081059, majf=18475, minf=28177648
cpu : usr=69.69%, sys=654.13%, ctx=145476035, majf=15867, minf=26176375
With request-based IO path:
seq-read : io=2048.0MB, bw=91074KB/s, iops=182147 , runt= 23027msec
seq-write : io=2048.0MB, bw=80725KB/s, iops=161449 , runt= 25979msec
rand-read : io=3095.7MB, bw=92106KB/s, iops=184211 , runt= 34416msec
rand-write: io=3095.7MB, bw=82815KB/s, iops=165630 , runt= 38277msec
clat (usec): min=0 , max=1932.4K, avg=77824.17, stdev=170339.49
clat (usec): min=0 , max=2510.2K, avg=78023.96, stdev=146949.15
clat (usec): min=0 , max=3037.2K, avg=74746.53, stdev=128498.27
clat (usec): min=0 , max=1363.4K, avg=89830.75, stdev=114279.68
cpu : usr=53.28%, sys=724.19%, ctx=37988895, majf=17531, minf=23577622
cpu : usr=49.03%, sys=633.20%, ctx=205935380, majf=18197, minf=27288959
cpu : usr=55.78%, sys=722.40%, ctx=101525058, majf=19273, minf=28067082
cpu : usr=56.55%, sys=690.83%, ctx=228205022, majf=18039, minf=26551985
2) Fio test is performed in a 8 vcpu guest with Fusion-IO based guest using
kvm tool.
Short version:
With bio-based IO path, sequential read/write, random read/write
IOPS boost : 11%, 11%, 13%, 10%
Latency improvement: 10%, 10%, 12%, 10%
Long Version:
With bio-based IO path:
read : io=2048.0MB, bw=58920KB/s, iops=117840 , runt= 35593msec
write: io=2048.0MB, bw=64308KB/s, iops=128616 , runt= 32611msec
read : io=3095.7MB, bw=59633KB/s, iops=119266 , runt= 53157msec
write: io=3095.7MB, bw=62993KB/s, iops=125985 , runt= 50322msec
clat (usec): min=0 , max=1284.3K, avg=128109.01, stdev=71513.29
clat (usec): min=94 , max=962339 , avg=116832.95, stdev=65836.80
clat (usec): min=0 , max=1846.6K, avg=128509.99, stdev=89575.07
clat (usec): min=0 , max=2256.4K, avg=121361.84, stdev=82747.25
cpu : usr=56.79%, sys=421.70%, ctx=147335118, majf=21080, minf=19852517
cpu : usr=61.81%, sys=455.53%, ctx=143269950, majf=16027, minf=24800604
cpu : usr=63.10%, sys=455.38%, ctx=178373538, majf=16958, minf=24822612
cpu : usr=62.04%, sys=453.58%, ctx=226902362, majf=16089, minf=23278105
With request-based IO path:
read : io=2048.0MB, bw=52896KB/s, iops=105791 , runt= 39647msec
write: io=2048.0MB, bw=57856KB/s, iops=115711 , runt= 36248msec
read : io=3095.7MB, bw=52387KB/s, iops=104773 , runt= 60510msec
write: io=3095.7MB, bw=57310KB/s, iops=114619 , runt= 55312msec
clat (usec): min=0 , max=1532.6K, avg=142085.62, stdev=109196.84
clat (usec): min=0 , max=1487.4K, avg=129110.71, stdev=114973.64
clat (usec): min=0 , max=1388.6K, avg=145049.22, stdev=107232.55
clat (usec): min=0 , max=1465.9K, avg=133585.67, stdev=110322.95
cpu : usr=44.08%, sys=590.71%, ctx=451812322, majf=14841, minf=17648641
cpu : usr=48.73%, sys=610.78%, ctx=418953997, majf=22164, minf=26850689
cpu : usr=45.58%, sys=581.16%, ctx=714079216, majf=21497, minf=22558223
cpu : usr=48.40%, sys=599.65%, ctx=656089423, majf=16393, minf=23824409
3) Fio test is performed in a 8 vcpu guest with normal SATA based guest
using kvm tool.
Short version:
With bio-based IO path, sequential read/write, random read/write
IOPS boost : -10%, -10%, 4.4%, 0.5%
Latency improvement: -12%, -15%, 2.5%, 0.8%
Long Version:
With bio-based IO path:
read : io=124812KB, bw=36537KB/s, iops=9060 , runt= 3416msec
write: io=169180KB, bw=24406KB/s, iops=6065 , runt= 6932msec
read : io=256200KB, bw=2089.3KB/s, iops=520 , runt=122630msec
write: io=257988KB, bw=1545.7KB/s, iops=384 , runt=166910msec
clat (msec): min=1 , max=1527 , avg=28.06, stdev=89.54
clat (msec): min=2 , max=344 , avg=41.12, stdev=38.70
clat (msec): min=8 , max=1984 , avg=490.63, stdev=207.28
clat (msec): min=33 , max=4131 , avg=659.19, stdev=304.71
cpu : usr=4.85%, sys=17.15%, ctx=31593, majf=0, minf=7
cpu : usr=3.04%, sys=11.45%, ctx=39377, majf=0, minf=0
cpu : usr=0.47%, sys=1.59%, ctx=262986, majf=0, minf=16
cpu : usr=0.47%, sys=1.46%, ctx=337410, majf=0, minf=0
With request-based IO path:
read : io=150120KB, bw=40420KB/s, iops=10037 , runt= 3714msec
write: io=194932KB, bw=27029KB/s, iops=6722 , runt= 7212msec
read : io=257136KB, bw=2001.1KB/s, iops=498 , runt=128443msec
write: io=258276KB, bw=1537.2KB/s, iops=382 , runt=168028msec
clat (msec): min=1 , max=1542 , avg=24.84, stdev=32.45
clat (msec): min=3 , max=628 , avg=35.62, stdev=39.71
clat (msec): min=8 , max=2540 , avg=503.28, stdev=236.97
clat (msec): min=41 , max=4398 , avg=653.88, stdev=302.61
cpu : usr=3.91%, sys=15.75%, ctx=26968, majf=0, minf=23
cpu : usr=2.50%, sys=10.56%, ctx=19090, majf=0, minf=0
cpu : usr=0.16%, sys=0.43%, ctx=20159, majf=0, minf=16
cpu : usr=0.18%, sys=0.53%, ctx=81364, majf=0, minf=0
How to use:
-----------------------------
Add 'virtio_blk.use_bio=1' to kernel cmdline or 'modprobe virtio_blk
use_bio=1' to enable ->make_request_fn() based I/O path.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Tejun Heo <tj@kernel.org>
Cc: Shaohua Li <shli@kernel.org>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Minchan Kim <minchan.kim@gmail.com>
Signed-off-by: Asias He <asias@redhat.com>
Acked-by: Rusty Russell <rusty@rustcorp.com.au>
---
drivers/block/virtio_blk.c | 203 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 163 insertions(+), 40 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c0bbeb4..95cfeed 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -14,6 +14,9 @@
#define PART_BITS 4
+static bool use_bio;
+module_param(use_bio, bool, S_IRUGO);
+
static int major;
static DEFINE_IDA(vd_index_ida);
@@ -23,6 +26,7 @@ struct virtio_blk
{
struct virtio_device *vdev;
struct virtqueue *vq;
+ wait_queue_head_t queue_wait;
/* The disk structure for the kernel. */
struct gendisk *disk;
@@ -51,53 +55,87 @@ struct virtio_blk
struct virtblk_req
{
struct request *req;
+ struct bio *bio;
struct virtio_blk_outhdr out_hdr;
struct virtio_scsi_inhdr in_hdr;
u8 status;
+ struct scatterlist sg[];
};
-static void blk_done(struct virtqueue *vq)
+static inline int virtblk_result(struct virtblk_req *vbr)
+{
+ switch (vbr->status) {
+ case VIRTIO_BLK_S_OK:
+ return 0;
+ case VIRTIO_BLK_S_UNSUPP:
+ return -ENOTTY;
+ default:
+ return -EIO;
+ }
+}
+
+static inline void virtblk_request_done(struct virtio_blk *vblk,
+ struct virtblk_req *vbr)
+{
+ struct request *req = vbr->req;
+ int error = virtblk_result(vbr);
+
+ if (req->cmd_type == REQ_TYPE_BLOCK_PC) {
+ req->resid_len = vbr->in_hdr.residual;
+ req->sense_len = vbr->in_hdr.sense_len;
+ req->errors = vbr->in_hdr.errors;
+ } else if (req->cmd_type == REQ_TYPE_SPECIAL) {
+ req->errors = (error != 0);
+ }
+
+ __blk_end_request_all(req, error);
+ mempool_free(vbr, vblk->pool);
+}
+
+static inline void virtblk_bio_done(struct virtio_blk *vblk,
+ struct virtblk_req *vbr)
+{
+ bio_endio(vbr->bio, virtblk_result(vbr));
+ mempool_free(vbr, vblk->pool);
+}
+
+static void virtblk_done(struct virtqueue *vq)
{
struct virtio_blk *vblk = vq->vdev->priv;
+ unsigned long bio_done = 0, req_done = 0;
struct virtblk_req *vbr;
- unsigned int len;
unsigned long flags;
+ unsigned int len;
spin_lock_irqsave(vblk->disk->queue->queue_lock, flags);
while ((vbr = virtqueue_get_buf(vblk->vq, &len)) != NULL) {
- int error;
-
- switch (vbr->status) {
- case VIRTIO_BLK_S_OK:
- error = 0;
- break;
- case VIRTIO_BLK_S_UNSUPP:
- error = -ENOTTY;
- break;
- default:
- error = -EIO;
- break;
- }
-
- switch (vbr->req->cmd_type) {
- case REQ_TYPE_BLOCK_PC:
- vbr->req->resid_len = vbr->in_hdr.residual;
- vbr->req->sense_len = vbr->in_hdr.sense_len;
- vbr->req->errors = vbr->in_hdr.errors;
- break;
- case REQ_TYPE_SPECIAL:
- vbr->req->errors = (error != 0);
- break;
- default:
- break;
+ if (vbr->bio) {
+ virtblk_bio_done(vblk, vbr);
+ bio_done++;
+ } else {
+ virtblk_request_done(vblk, vbr);
+ req_done++;
}
-
- __blk_end_request_all(vbr->req, error);
- mempool_free(vbr, vblk->pool);
}
/* In case queue is stopped waiting for more buffers. */
- blk_start_queue(vblk->disk->queue);
+ if (req_done)
+ blk_start_queue(vblk->disk->queue);
spin_unlock_irqrestore(vblk->disk->queue->queue_lock, flags);
+
+ if (bio_done)
+ wake_up(&vblk->queue_wait);
+}
+
+static inline struct virtblk_req *virtblk_alloc_req(struct virtio_blk *vblk,
+ gfp_t gfp_mask)
+{
+ struct virtblk_req *vbr;
+
+ vbr = mempool_alloc(vblk->pool, gfp_mask);
+ if (vbr && use_bio)
+ sg_init_table(vbr->sg, vblk->sg_elems);
+
+ return vbr;
}
static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
@@ -106,13 +144,13 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
unsigned long num, out = 0, in = 0;
struct virtblk_req *vbr;
- vbr = mempool_alloc(vblk->pool, GFP_ATOMIC);
+ vbr = virtblk_alloc_req(vblk, GFP_ATOMIC);
if (!vbr)
/* When another request finishes we'll try again. */
return false;
vbr->req = req;
-
+ vbr->bio = NULL;
if (req->cmd_flags & REQ_FLUSH) {
vbr->out_hdr.type = VIRTIO_BLK_T_FLUSH;
vbr->out_hdr.sector = 0;
@@ -172,7 +210,8 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
}
}
- if (virtqueue_add_buf(vblk->vq, vblk->sg, out, in, vbr, GFP_ATOMIC)<0) {
+ if (virtqueue_add_buf(vblk->vq, vblk->sg, out, in, vbr,
+ GFP_ATOMIC) < 0) {
mempool_free(vbr, vblk->pool);
return false;
}
@@ -180,7 +219,7 @@ static bool do_req(struct request_queue *q, struct virtio_blk *vblk,
return true;
}
-static void do_virtblk_request(struct request_queue *q)
+static void virtblk_request(struct request_queue *q)
{
struct virtio_blk *vblk = q->queuedata;
struct request *req;
@@ -203,6 +242,82 @@ static void do_virtblk_request(struct request_queue *q)
virtqueue_kick(vblk->vq);
}
+static void virtblk_add_buf_wait(struct virtio_blk *vblk,
+ struct virtblk_req *vbr,
+ unsigned long out,
+ unsigned long in)
+{
+ DEFINE_WAIT(wait);
+
+ for (;;) {
+ prepare_to_wait_exclusive(&vblk->queue_wait, &wait,
+ TASK_UNINTERRUPTIBLE);
+
+ spin_lock_irq(vblk->disk->queue->queue_lock);
+ if (virtqueue_add_buf(vblk->vq, vbr->sg, out, in, vbr,
+ GFP_ATOMIC) < 0) {
+ spin_unlock_irq(vblk->disk->queue->queue_lock);
+ io_schedule();
+ } else {
+ virtqueue_kick(vblk->vq);
+ spin_unlock_irq(vblk->disk->queue->queue_lock);
+ break;
+ }
+
+ }
+
+ finish_wait(&vblk->queue_wait, &wait);
+}
+
+static void virtblk_make_request(struct request_queue *q, struct bio *bio)
+{
+ struct virtio_blk *vblk = q->queuedata;
+ unsigned int num, out = 0, in = 0;
+ struct virtblk_req *vbr;
+
+ BUG_ON(bio->bi_phys_segments + 2 > vblk->sg_elems);
+ BUG_ON(bio->bi_rw & (REQ_FLUSH | REQ_FUA));
+
+ vbr = virtblk_alloc_req(vblk, GFP_NOIO);
+ if (!vbr) {
+ bio_endio(bio, -ENOMEM);
+ return;
+ }
+
+ vbr->bio = bio;
+ vbr->req = NULL;
+ vbr->out_hdr.type = 0;
+ vbr->out_hdr.sector = bio->bi_sector;
+ vbr->out_hdr.ioprio = bio_prio(bio);
+
+ sg_set_buf(&vbr->sg[out++], &vbr->out_hdr, sizeof(vbr->out_hdr));
+
+ num = blk_bio_map_sg(q, bio, vbr->sg + out);
+
+ sg_set_buf(&vbr->sg[num + out + in++], &vbr->status,
+ sizeof(vbr->status));
+
+ if (num) {
+ if (bio->bi_rw & REQ_WRITE) {
+ vbr->out_hdr.type |= VIRTIO_BLK_T_OUT;
+ out += num;
+ } else {
+ vbr->out_hdr.type |= VIRTIO_BLK_T_IN;
+ in += num;
+ }
+ }
+
+ spin_lock_irq(vblk->disk->queue->queue_lock);
+ if (unlikely(virtqueue_add_buf(vblk->vq, vbr->sg, out, in, vbr,
+ GFP_ATOMIC) < 0)) {
+ spin_unlock_irq(vblk->disk->queue->queue_lock);
+ virtblk_add_buf_wait(vblk, vbr, out, in);
+ return;
+ }
+ virtqueue_kick(vblk->vq);
+ spin_unlock_irq(vblk->disk->queue->queue_lock);
+}
+
/* return id (s/n) string for *disk to *id_str
*/
static int virtblk_get_id(struct gendisk *disk, char *id_str)
@@ -360,7 +475,7 @@ static int init_vq(struct virtio_blk *vblk)
int err = 0;
/* We expect one virtqueue, for output. */
- vblk->vq = virtio_find_single_vq(vblk->vdev, blk_done, "requests");
+ vblk->vq = virtio_find_single_vq(vblk->vdev, virtblk_done, "requests");
if (IS_ERR(vblk->vq))
err = PTR_ERR(vblk->vq);
@@ -414,7 +529,7 @@ static void virtblk_update_cache_mode(struct virtio_device *vdev)
u8 writeback = virtblk_get_cache_mode(vdev);
struct virtio_blk *vblk = vdev->priv;
- if (writeback)
+ if (writeback && !use_bio)
blk_queue_flush(vblk->disk->queue, REQ_FLUSH);
else
blk_queue_flush(vblk->disk->queue, 0);
@@ -477,6 +592,8 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
struct virtio_blk *vblk;
struct request_queue *q;
int err, index;
+ int pool_size;
+
u64 cap;
u32 v, blk_size, sg_elems, opt_io_size;
u16 min_io_size;
@@ -506,10 +623,12 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
goto out_free_index;
}
+ init_waitqueue_head(&vblk->queue_wait);
vblk->vdev = vdev;
vblk->sg_elems = sg_elems;
sg_init_table(vblk->sg, vblk->sg_elems);
mutex_init(&vblk->config_lock);
+
INIT_WORK(&vblk->config_work, virtblk_config_changed_work);
vblk->config_enable = true;
@@ -517,7 +636,10 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
if (err)
goto out_free_vblk;
- vblk->pool = mempool_create_kmalloc_pool(1,sizeof(struct virtblk_req));
+ pool_size = sizeof(struct virtblk_req);
+ if (use_bio)
+ pool_size += sizeof(struct scatterlist) * sg_elems;
+ vblk->pool = mempool_create_kmalloc_pool(1, pool_size);
if (!vblk->pool) {
err = -ENOMEM;
goto out_free_vq;
@@ -530,12 +652,14 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
goto out_mempool;
}
- q = vblk->disk->queue = blk_init_queue(do_virtblk_request, NULL);
+ q = vblk->disk->queue = blk_init_queue(virtblk_request, NULL);
if (!q) {
err = -ENOMEM;
goto out_put_disk;
}
+ if (use_bio)
+ blk_queue_make_request(q, virtblk_make_request);
q->queuedata = vblk;
virtblk_name_format("vd", index, vblk->disk->disk_name, DISK_NAME_LEN);
@@ -620,7 +744,6 @@ static int __devinit virtblk_probe(struct virtio_device *vdev)
if (!err && opt_io_size)
blk_queue_io_opt(q, blk_size * opt_io_size);
-
add_disk(vblk->disk);
err = device_create_file(disk_to_dev(vblk->disk), &dev_attr_serial);
if (err)
--
1.7.11.2
^ permalink raw reply related
* [PATCH V7 0/2] Improve virtio-blk performance
From: Asias He @ 2012-08-08 8:07 UTC (permalink / raw)
To: linux-kernel
Cc: Jens Axboe, kvm, Michael S. Tsirkin, virtualization, Tejun Heo,
Shaohua Li, Christoph Hellwig
Hi, all
Changes in v7:
- Using vbr->flags to trace request type
- Dropped unnecessary struct virtio_blk *vblk parameter
- Reuse struct virtblk_req in bio done function
- Added performance data on normal SATA device and the reason why make it optional
Fio test shows bio-based IO path gives the following performance improvement:
1) Ramdisk device
With bio-based IO path, sequential read/write, random read/write
IOPS boost : 28%, 24%, 21%, 16%
Latency improvement: 32%, 17%, 21%, 16%
2) Fusion IO device
With bio-based IO path, sequential read/write, random read/write
IOPS boost : 11%, 11%, 13%, 10%
Latency improvement: 10%, 10%, 12%, 10%
3) Normal SATA device
With bio-based IO path, sequential read/write, random read/write
IOPS boost : -10%, -10%, 4.4%, 0.5%
Latency improvement: -12%, -15%, 2.5%, 0.8%
Asias He (2):
virtio-blk: Add bio-based IO path for virtio-blk
virtio-blk: Add REQ_FLUSH and REQ_FUA support to bio path
drivers/block/virtio_blk.c | 301 +++++++++++++++++++++++++++++++++++++++------
1 file changed, 264 insertions(+), 37 deletions(-)
--
1.7.11.2
^ permalink raw reply
* Re: [PATCH V6 0/2] Improve virtio-blk performance
From: Asias He @ 2012-08-08 6:24 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, kvm, Michael S. Tsirkin, linux-kernel, virtualization,
Tejun Heo, Shaohua Li
In-Reply-To: <5021D87C.4050400@redhat.com>
On 08/08/2012 11:09 AM, Asias He wrote:
> On 08/07/2012 05:16 PM, Christoph Hellwig wrote:
>> On Tue, Aug 07, 2012 at 04:47:13PM +0800, Asias He wrote:
>>> 1) Ramdisk device
>>> With bio-based IO path, sequential read/write, random read/write
>>> IOPS boost : 28%, 24%, 21%, 16%
>>> Latency improvement: 32%, 17%, 21%, 16%
>>> 2) Fusion IO device
>>> With bio-based IO path, sequential read/write, random read/write
>>> IOPS boost : 11%, 11%, 13%, 10%
>>> Latency improvement: 10%, 10%, 12%, 10%
>>
>> Do you also have numbers for normal SAS/SATA disks? The changelog should
>> have a reall good explanation of why this is optional, and numbers are a
>> very important part of that.
Yes. I posted the numbers on normal SATA disks a few days ago in the
thread with Sasha. Will add that data and explanation of why optional to
the changelog
--
Asias
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox