[PATCH 0/2] RFC: Implement I/O QoS in dom0

[PATCH 0/2] RFC: Implement I/O QoS in dom0

[William Pitcock]

Greetings,

This patch series against xen.git implements a nieve token-based QoS
limiter for I/O accesses using blkback. The idea is to ensure a fair
share of disk I/O resources to all guests on the machine. Right now
if you have a setup involving AoE, ensuring guest reliability is much
more complex because the AoE layer bypasses the system scheduler.

This patchset intends to add simple QoS limits. It was inspired by the
'token-limiter' patch for UML, which is quite effective for limiting
I/O usage. However, since the limits are enforced by the host, this means
that users cannot get around the QoS limits by using a different kernel.

This work is not yet complete, see my commit commentary for things that
need to be done.

Mostly right now we need to consider whether a sysctl interface or a
sysfs interface would be best to implement for tweaking the blkback
limits.

Also please note that all of this applies to paravirtualized guests only,
HVM guests go through qemu-dm, which can be effectively limited using
ionice.

Please also note that this is a test patch only, and is not the final
version. Commentary and discussion of this patch and ways that it could be
changed/improved would be desirable.

William Pitcock (2):
  blkback: Implement VBD QoS mechanics.
  blkback: Add default policy values for I/O QoS code

 drivers/xen/blkback/blkback.c   |   22 ++++++++++++++++++++++
 drivers/xen/blkback/common.h    |    6 ++++++
 drivers/xen/blkback/interface.c |    9 +++++++++
 3 files changed, 37 insertions(+), 0 deletions(-)

[PATCH 1/2] blkback: Implement VBD QoS mechanics.

[William Pitcock]

Impact: add ability to define I/O usage policies

This enables a nieve token-based QoS system which allows for guests to
get an equal share of I/O requests. Without this patch, a misbehaving or
misconfigured guest can use up any available I/O resources, degrading I/O
performance for other guests on the machine.

The method for specifying these limitations is not implemented in this patch,
because that is an operation detail.

Signed-off-by: William Pitcock <nenolod@dereferenced.org>
---
 drivers/xen/blkback/blkback.c |   22 ++++++++++++++++++++++
 drivers/xen/blkback/common.h  |    6 ++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/blkback/blkback.c b/drivers/xen/blkback/blkback.c
index 8d988f4..996869e 100644
--- a/drivers/xen/blkback/blkback.c
+++ b/drivers/xen/blkback/blkback.c
@@ -203,6 +203,14 @@ static void print_stats(blkif_t *blkif)
 	blkif->st_oo_req = 0;
 }
 
+static void refill_reqcount(blkif_t *blkif)
+{
+	blkif->reqtime = jiffies + msecs_to_jiffies(1000);
+	blkif->reqcount += blkif->reqrate;
+	if (blkif->reqcount > blkif->reqmax)
+		blkif->reqcount = blkif->reqmax;
+}
+
 int blkif_schedule(void *arg)
 {
 	blkif_t *blkif = arg;
@@ -232,6 +240,9 @@ int blkif_schedule(void *arg)
 
 		if (log_stats && time_after(jiffies, blkif->st_print))
 			print_stats(blkif);
+
+		if (time_after(jiffies, blkif->reqtime))
+			refill_reqcount(blkif);
 	}
 
 	if (log_stats)
@@ -313,11 +324,22 @@ static int do_block_io_op(blkif_t *blkif)
 	rp = blk_rings->common.sring->req_prod;
 	rmb(); /* Ensure we see queued requests up to 'rp'. */
 
+	/* if there's no available request tokens right now, and limiting
+	 * is requested, then don't bother with going any further. */
+	if (blkif->reqcount <= 0 && blkif->reqmax != 0)
+		return (rc != rp) ? 1 : 0;
+
 	while (rc != rp) {
 
 		if (RING_REQUEST_CONS_OVERFLOW(&blk_rings->common, rc))
 			break;
 
+		/* FIXME: Should we report QoS overages as VBD_OO, or not? */
+		if (--blkif->reqcount <= 0) {
+			more_to_do = 1;
+			break;
+		}
+
 		if (kthread_should_stop()) {
 			more_to_do = 1;
 			break;
diff --git a/drivers/xen/blkback/common.h b/drivers/xen/blkback/common.h
index 57b7825..45af42f 100644
--- a/drivers/xen/blkback/common.h
+++ b/drivers/xen/blkback/common.h
@@ -92,6 +92,12 @@ typedef struct blkif_st {
 
 	grant_handle_t shmem_handle;
 	grant_ref_t    shmem_ref;
+
+	/* qos information */
+	long		    reqtime;
+	int		    reqcount;
+	int		    reqmax;
+	int		    reqfill;
 } blkif_t;
 
 blkif_t *blkif_alloc(domid_t domid);
-- 
1.6.1.3

[PATCH 2/2] blkback: Add default policy values for I/O QoS code

[William Pitcock]

This patch adds default values which define a default QoS policy for
guest I/O. A sysctl interface will be added once /proc/sys/xen is available
again... unless I come up with a better idea in the meantime.

Ideas?

Signed-off-by: William Pitcock <nenolod@dereferenced.org>
---
 drivers/xen/blkback/interface.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/xen/blkback/interface.c b/drivers/xen/blkback/interface.c
index c6c3e14..9541ec4 100644
--- a/drivers/xen/blkback/interface.c
+++ b/drivers/xen/blkback/interface.c
@@ -34,9 +34,13 @@
 #include <xen/events.h>
 #include <xen/grant_table.h>
 #include <linux/kthread.h>
+#include <linux/sysctl.h>
 
 static struct kmem_cache *blkif_cachep;
 
+static int blkif_reqmax = 500000;
+static int blkif_reqrate = 512;
+
 blkif_t *blkif_alloc(domid_t domid)
 {
 	blkif_t *blkif;
@@ -53,6 +57,11 @@ blkif_t *blkif_alloc(domid_t domid)
 	blkif->st_print = jiffies;
 	init_waitqueue_head(&blkif->waiting_to_free);
 
+	/* initialize QoS values to defaults */
+	blkif->reqrate = blkif_reqrate;
+	blkif->reqmax = blkif_reqmax;
+	blkif->reqcount = blkif->reqmax;
+
 	return blkif;
 }
 
-- 
1.6.1.3

Re: [PATCH 2/2] blkback: Add default policy values for I/O QoS code

[Jeremy Fitzhardinge]

William Pitcock wrote:
> This patch adds default values which define a default QoS policy for
> guest I/O. A sysctl interface will be added once /proc/sys/xen is available
> again... unless I come up with a better idea in the meantime.
>   

What would that interface look like?  Would the parameters be per 
domain, or per device?  Would sysfs be suitable?

    J
> Ideas?
>
> Signed-off-by: William Pitcock <nenolod@dereferenced.org>
> ---
>  drivers/xen/blkback/interface.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/xen/blkback/interface.c b/drivers/xen/blkback/interface.c
> index c6c3e14..9541ec4 100644
> --- a/drivers/xen/blkback/interface.c
> +++ b/drivers/xen/blkback/interface.c
> @@ -34,9 +34,13 @@
>  #include <xen/events.h>
>  #include <xen/grant_table.h>
>  #include <linux/kthread.h>
> +#include <linux/sysctl.h>
>  
>  static struct kmem_cache *blkif_cachep;
>  
> +static int blkif_reqmax = 500000;
> +static int blkif_reqrate = 512;
> +
>  blkif_t *blkif_alloc(domid_t domid)
>  {
>  	blkif_t *blkif;
> @@ -53,6 +57,11 @@ blkif_t *blkif_alloc(domid_t domid)
>  	blkif->st_print = jiffies;
>  	init_waitqueue_head(&blkif->waiting_to_free);
>  
> +	/* initialize QoS values to defaults */
> +	blkif->reqrate = blkif_reqrate;
> +	blkif->reqmax = blkif_reqmax;
> +	blkif->reqcount = blkif->reqmax;
> +
>  	return blkif;
>  }
>  
>   

Re: [PATCH 2/2] blkback: Add default policy values for I/O QoS code

[Ian Campbell]

On Tue, 2009-03-31 at 03:22 -0400, Jeremy Fitzhardinge wrote:
> William Pitcock wrote:
> > This patch adds default values which define a default QoS policy for
> > guest I/O. A sysctl interface will be added once /proc/sys/xen is available
> > again... unless I come up with a better idea in the meantime.
> >   
> 
> What would that interface look like?  Would the parameters be per 
> domain, or per device?  Would sysfs be suitable?

The netback interface to the similar thing is via xenstore, FWIW.

Ian.

> 
>     J
> > Ideas?
> >
> > Signed-off-by: William Pitcock <nenolod@dereferenced.org>
> > ---
> >  drivers/xen/blkback/interface.c |    9 +++++++++
> >  1 files changed, 9 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/xen/blkback/interface.c b/drivers/xen/blkback/interface.c
> > index c6c3e14..9541ec4 100644
> > --- a/drivers/xen/blkback/interface.c
> > +++ b/drivers/xen/blkback/interface.c
> > @@ -34,9 +34,13 @@
> >  #include <xen/events.h>
> >  #include <xen/grant_table.h>
> >  #include <linux/kthread.h>
> > +#include <linux/sysctl.h>
> >  
> >  static struct kmem_cache *blkif_cachep;
> >  
> > +static int blkif_reqmax = 500000;
> > +static int blkif_reqrate = 512;
> > +
> >  blkif_t *blkif_alloc(domid_t domid)
> >  {
> >  	blkif_t *blkif;
> > @@ -53,6 +57,11 @@ blkif_t *blkif_alloc(domid_t domid)
> >  	blkif->st_print = jiffies;
> >  	init_waitqueue_head(&blkif->waiting_to_free);
> >  
> > +	/* initialize QoS values to defaults */
> > +	blkif->reqrate = blkif_reqrate;
> > +	blkif->reqmax = blkif_reqmax;
> > +	blkif->reqcount = blkif->reqmax;
> > +
> >  	return blkif;
> >  }
> >  
> >   
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH 2/2] blkback: Add default policy values for I/O QoS code

[William Pitcock]

On Tue, 2009-03-31 at 00:22 -0700, Jeremy Fitzhardinge wrote:
> William Pitcock wrote:
> > This patch adds default values which define a default QoS policy for
> > guest I/O. A sysctl interface will be added once /proc/sys/xen is available
> > again... unless I come up with a better idea in the meantime.
> >   
> 
> What would that interface look like?  Would the parameters be per 
> domain, or per device?  Would sysfs be suitable?

I was thinking about sysfs as an option, but xenstore also seems
desirable, since that way we can integrate QoS policy directly into the
vm config in an easy way without having to modify the userland tools in
any sort of major way.

Ian mentioned that netback uses xenstore for getting information, so I
think we could do the same here to get the QoS policy. It seems like a
sensible way.

Also, hopefully the dom0 base stuff will make it for 2.6.30. Can't wait
to have pvops-dom0 in production... keeping my fingers crossed on that
one. Although I could just backport this work to 2.6.18-xen.hg if it
comes down to being necessary to do so.

William