Re:[PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

Re:[PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Satoshi Uchida]

[-- Attachment #1.1: Type: text/plain, Size: 715 bytes --]

 
Hi, Keir.

>>
>> This patch add /xen/proc/vbd.
>> This proc file stores counts of I/O requests for VBD.
>> I/O requests is counted for each Domains.
>
>We keep getting complained at for adding stuff to /proc. Shouldn't this 
>go
under the blkback's sysfs directory somewhere?
>
>  -- Keir

Uom.

In my subjective, currently, procfs is used for representing dynamic
information such as statistical data, and sysfs is used for representing
statical information such as module parameters.
In this case, I think that this I/O information is  former, so information
is put into procfs in this time.
As similar example, ballon infromation exist in procfs.

Is it better that realizing at sysfs?

Thanks,
Satoshi UCHIDA

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4036 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Anthony Liguori]

Satoshi Uchida wrote:
> Uom.
>
> In my subjective, currently, procfs is used for representing dynamic
> information such as statistical data, and sysfs is used for representing
> statical information such as module parameters.
>   

/proc is for process information.  This stuff definitely belongs in 
sysfs.  It's analogous to /sys/class/net/eth0/statistics/.

> In this case, I think that this I/O information is  former, so information
> is put into procfs in this time.
> As similar example, ballon infromation exist in procfs.
>   

The balloon driver shouldn't be using proc.

Regards,

Anthony Liguori

> Is it better that realizing at sysfs?
>
> Thanks,
> Satoshi UCHIDA
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>   

Re: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Keir Fraser]

On 21 Jun 2006, at 12:47, Anthony Liguori wrote:

>> In my subjective, currently, procfs is used for representing dynamic
>> information such as statistical data, and sysfs is used for 
>> representing
>> statical information such as module parameters.
>
> /proc is for process information.  This stuff definitely belongs in 
> sysfs.  It's analogous to /sys/class/net/eth0/statistics/.

Yes. The patchset looks good and is obviously useful -- we'd like to 
apply it if this gets fixed.

  -- Keir

Re: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Anthony Liguori]

On Wed, 21 Jun 2006 13:30:34 +0100, Keir Fraser wrote:

> 
> On 21 Jun 2006, at 12:47, Anthony Liguori wrote:
> 
>>> In my subjective, currently, procfs is used for representing dynamic
>>> information such as statistical data, and sysfs is used for 
>>> representing
>>> statical information such as module parameters.
>>
>> /proc is for process information.  This stuff definitely belongs in 
>> sysfs.  It's analogous to /sys/class/net/eth0/statistics/.
> 
> Yes. The patchset looks good and is obviously useful -- we'd like to 
> apply it if this gets fixed.

Agreed!  It's a very common feature request so it'll be great to get this
in the tree.  This information is sort of available in
/sys/block/<backend-dev>/stat but it's not terribly reliable in my
experience (it's always zero's for loop devices IIRC).

These patches are missing Signed-Off-By lines too btw.  When you resubmit,
make sure to include those.

Thanks,

Anthony Liguori

>   -- Keir

RE: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Satoshi Uchida]

 
Hi,

> I looked at the code, and it seems "oo" stands of "out of", which  
> indicates how much we were out of request (Xen supports up to 64 
> requests in queue)

Thanks.
It may be correct.

>Few minor comments:
>- The variable dom_io_info_wq is unused, should be removed.
>- printk("%s: out of memory\n", __FUNCTION__) should be used with 
>KERN_ERR

Thanks.
I want to review around these codes.


>I have one question: in your below output:
>
>> Domain 4 - oo        0  rd    18699  wr        2
>>  xvd 4  8:21 - oo        0  rd    18699  wr        2
>
>I notice that you output the domid with xvd (4), but in the above line, we already had that information (Domain 4).
> So why you need to show the domid again?

VBD processes in domain0 runs as xvd threads.
These threads names "xvd [Domain name] [Major Number]:[Minor Number]".
So, I show same name inelligibly.
In addition, I think that total Information for each domain is added optionally.
Therefore, its informatin is needed in such time.


Thanks,
Satoshi UCHIDA

Re: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Satoshi Uchida]

Thanks, Feir and Anthony.

> On 21 Jun 2006, at 12:47, Anthony Liguori wrote:
>
>>> In my subjective, currently, procfs is used for representing dynamic 
>>> information such as statistical data, and sysfs is used for 
>>> representing statical information such as module parameters.
>>
>> /proc is for process information.  This stuff definitely belongs in 
>> sysfs.  It's analogous to /sys/class/net/eth0/statistics/.
>
>Yes. The patchset looks good and is obviously useful -- we'd like to apply it if this gets fixed.
>
> -- Keir

It's OK.
I try to examine and develop transition to sysfs file system.

I may worry that where does information locate in sysfs.
In it case, I confer with you, so please give me nice answer.
Current, I think two pattern.
  1. vbd class create /sysfs/class.
  2. Use vbd-* directory under /sysfs/devices/xen-backend/.
May first idea be appropriate?

In addition,
The one of advantage for our method is to represent total information for domain.
This mechanism is useful for analysis for each domain I/O or total environments.
In sysfs, I see that Interface connect individual device/module information.
Naturally, it is enough by using xentop, but xentop can not take log file.
If xentop can be redirected to normal file, but it file insert binary code.
Aren't there any good ideas?

Thanks,
Satoshi UCHIDA

Re: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Keir Fraser]

On 22 Jun 2006, at 01:54, Satoshi Uchida wrote:

> I may worry that where does information locate in sysfs.
> In it case, I confer with you, so please give me nice answer.
> Current, I think two pattern.
>   1. vbd class create /sysfs/class.
>   2. Use vbd-* directory under /sysfs/devices/xen-backend/.
> May first idea be appropriate?

I think the info should be placed in the directories you mention in 
(2). If there is a 'block class' later then the vbd directories will 
symlink to (1) anyway (just as happens for vifs), so you can't really 
go wrong.

  -- Keir

[PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Satoshi Uchida]

[-- Attachment #1: Type: text/plain, Size: 1242 bytes --]

 
This patch add /xen/proc/vbd.
This proc file stores counts of I/O requests for VBD.
I/O requests is counted for each Domains.

For example, in case of four VMs, outputs of reading /xen/proc/vbd are as follows.

root@dom0 ~]# more /proc/xen/vbd
Domain 4 - oo        0  rd    18699  wr        2
 xvd 4  8:21 - oo        0  rd    18699  wr        2
Domain 7 - oo        0  rd      869  wr      376
 xvd 7  8:22 - oo        0  rd      869  wr      376
Domain 8 - oo        0  rd      721  wr      197
 xvd 8  8:23 - oo        0  rd      721  wr      197
Domain 9 - oo        0  rd      640  wr       77
 xvd 9  8:24 - oo        0  rd      640  wr       77

The outputs show the following information.

The first line shows domain information.
     Detail: Domain [Domain ID] - oo [oo-reqs] rd  [rd-reqs] wr [wr-reqs].
The second or less than third line shows each VBD information.
        xvd [Domain ID] [device majar]:[device minor]  - oo [oo-reqs] rd  [rd-reqs] wr [wr-reqs].

VBD information handles "xvd [Domain ID] [device majar]:[device minor]" for reason
 that the thread is running same name.

Currently, Domain show domain ID.
Someday, it want to correspond with domain name.


Thanks,
Satoshi UCHIDA

[-- Attachment #2: proc_xen_vbd.patch --]
[-- Type: application/octet-stream, Size: 7360 bytes --]

# HG changeset patch
# User s-uchida@ap.jp.nec.com
# Node ID 55ab88937d77290a54d3315ea4cab6234efd4e78
# Parent  360f9dc71f513c5e371c8c1b09cbb0f3c8e34d63
Add /proc/xen/vbd.  Output vbd request information.

diff -r 360f9dc71f51 -r 55ab88937d77 linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c
--- a/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c	Tue Jun 13 11:41:15 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c	Thu Jun 15 13:57:11 2006 +0900
@@ -41,6 +41,10 @@
 #include <xen/balloon.h>
 #include <asm/hypervisor.h>
 #include "common.h"
+
+#include <linux/kernel.h>
+#include <xen/xen_proc.h>
+#include <xen/xenbus.h>
 
 /*
  * These are rather arbitrary. They are fairly large because adjacent requests
@@ -85,6 +89,30 @@ static DEFINE_SPINLOCK(pending_free_lock
 static DEFINE_SPINLOCK(pending_free_lock);
 static DECLARE_WAIT_QUEUE_HEAD(pending_free_wq);
 
+/*
+ * VBD(Virtual Block Device) information is allocated to 'dom_io_info'.
+ * by each domain.
+ */
+
+typedef struct {
+	domid_t        dom;
+	unsigned long long rd_req;
+	unsigned long long wr_req;
+	unsigned long long oo_req;
+	struct list_head blkif_list;
+	struct list_head sibling_list;
+} dom_io_info_t;
+
+static struct list_head dom_io_info;
+static spinlock_t dom_io_info_lock = SPIN_LOCK_UNLOCKED;
+static DECLARE_WAIT_QUEUE_HEAD(dom_io_info_wq);
+
+/*
+ * VBD information access proc file system which located /proc/xen/vbd.
+ */
+static struct proc_dir_entry *blkback_pde;
+
+
 #define BLKBACK_INVALID_HANDLE (~0)
 
 static unsigned long mmap_vstart;
@@ -185,6 +213,145 @@ static void fast_flush_area(pending_req_
 	BUG_ON(ret);
 }
 
+
+/********************************************
+ * PROC OUTPUT FUNCTIONS
+ */
+static int put_proc_domain(char *page, dom_io_info_t *info)
+{
+	blkif_t *blkif;
+	int len;
+
+
+	info->oo_req = 0;
+	info->rd_req = 0;
+	info->wr_req = 0;
+
+	list_for_each_entry(blkif, &info->blkif_list, sibling_list) {
+		info->oo_req += blkif->st_oo_req;
+		info->rd_req += blkif->st_rd_req;
+		info->wr_req += blkif->st_wr_req;
+	}
+		
+	len = sprintf(
+		page,
+		"Domain %u - oo %8llu  rd %8llu  wr %8llu"
+		"\n",
+		info->dom, info->oo_req,
+		info->rd_req, info->wr_req
+		);
+
+	list_for_each_entry(blkif, &info->blkif_list, sibling_list) {
+      		len += sprintf(
+			page + len, 
+			" xvd %u %2u:%2u - oo %8d  rd %8d  wr %8d"
+			"\n",
+			blkif->domid,
+			blkif->be->major, blkif->be->minor,
+			blkif->st_oo_req, blkif->st_rd_req, blkif->st_wr_req,
+		      );
+	}
+
+	return len;
+}
+
+static int put_proc_domains(char *page)
+{
+	dom_io_info_t *info;
+	unsigned long flags;
+	int len;
+
+	len = 0;
+	spin_lock_irqsave(&dom_io_info_lock, flags);
+	list_for_each_entry(info, &dom_io_info, sibling_list) {
+	        len += put_proc_domain(page+len, info);
+	}
+	spin_unlock_irqrestore(&dom_io_info_lock, flags);
+
+	return len;
+}
+
+
+static int blkback_read(char *page, char **start, off_t off,
+		       int count, int *eof, void *data)
+{
+	int len;
+
+	len = put_proc_domains(page);
+	
+	*eof = 1;
+	return len;
+}
+
+
+/*****************************************************************
+ * VBD information structure control
+ */
+
+dom_io_info_t *dominfo_get(domid_t dom)
+{
+	dom_io_info_t *info;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dom_io_info_lock, flags);
+	list_for_each_entry(info, &dom_io_info, sibling_list) {
+		if (info->dom == dom) {
+			spin_unlock_irqrestore(&dom_io_info_lock, flags);
+			return info;
+		}
+	}
+	spin_unlock_irqrestore(&dom_io_info_lock, flags);
+
+	return NULL;
+}
+
+
+static int info_add(blkif_t *blkif)
+{
+	dom_io_info_t *info;
+
+	if (!(info = dominfo_get(blkif->domid))) {
+		info = kmalloc(sizeof(*info), GFP_KERNEL);
+		if (!info) {
+			printk("%s: out of memory\n", __FUNCTION__);
+			return -ENOMEM;
+		}
+		info->dom = blkif->domid ;
+		INIT_LIST_HEAD(&info->blkif_list);
+		info->rd_req = 0;
+		info->wr_req = 0;
+		info->oo_req = 0;
+		list_add_tail(&info->sibling_list, &dom_io_info);
+	}
+
+	list_add_tail(&blkif->sibling_list, &info->blkif_list);
+
+	return 0;
+}
+
+
+static int info_del(blkif_t *blkif)
+{
+	dom_io_info_t *info;
+	unsigned long flags;
+
+	if (!(info = dominfo_get(blkif->domid))) {
+		return -1;
+	}
+
+	spin_lock_irqsave(&dom_io_info_lock, flags);
+	list_del(&blkif->sibling_list);
+	
+	if (list_empty(&info->blkif_list)) {
+		list_del(&info->sibling_list);
+		kfree(info);
+	}
+	spin_unlock_irqrestore(&dom_io_info_lock, flags);
+
+	return 0;
+}
+
+
 /******************************************************************
  * SCHEDULER FUNCTIONS
  */
@@ -209,6 +376,8 @@ int blkif_schedule(void *arg)
 	if (debug_lvl)
 		printk(KERN_DEBUG "%s: started\n", current->comm);
 
+	info_add(blkif);
+
 	while (!kthread_should_stop()) {
 		wait_event_interruptible(
 			blkif->wq,
@@ -232,6 +401,8 @@ int blkif_schedule(void *arg)
 		print_stats(blkif);
 	if (debug_lvl)
 		printk(KERN_DEBUG "%s: exiting\n", current->comm);
+
+	info_del(blkif);
 
 	blkif->xenblkd = NULL;
 	blkif_put(blkif);
@@ -554,12 +725,22 @@ static int __init blkif_init(void)
 
 	memset(pending_reqs, 0, sizeof(pending_reqs));
 	INIT_LIST_HEAD(&pending_free);
+	INIT_LIST_HEAD(&dom_io_info);
 
 	for (i = 0; i < blkif_reqs; i++)
 		list_add_tail(&pending_reqs[i].free_list, &pending_free);
     
 	blkif_xenbus_init();
 
+	/* set up /proc/xen/vbd */
+	if ((blkback_pde = create_xen_proc_entry("vbd", 0444)) == NULL) {
+		DPRINTK("xen_vbd: Unable to create /proc/xen/vbd.\n");
+		return -1;
+	}
+
+	blkback_pde->read_proc  = blkback_read;
+
+
 	return 0;
 }
 
diff -r 360f9dc71f51 -r 55ab88937d77 linux-2.6-xen-sparse/drivers/xen/blkback/common.h
--- a/linux-2.6-xen-sparse/drivers/xen/blkback/common.h	Tue Jun 13 11:41:15 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/blkback/common.h	Thu Jun 15 13:57:11 2006 +0900
@@ -44,6 +44,7 @@
 #include <xen/interface/io/ring.h>
 #include <xen/gnttab.h>
 #include <xen/driver_util.h>
+#include <xen/xenbus.h>
 
 #define DPRINTK(_f, _a...)			\
 	pr_debug("(file=%s, line=%d) " _f,	\
@@ -92,7 +93,19 @@ typedef struct blkif_st {
 
 	grant_handle_t shmem_handle;
 	grant_ref_t    shmem_ref;
+
+	struct list_head sibling_list;
 } blkif_t;
+
+struct backend_info
+{
+ 	struct xenbus_device *dev;
+ 	blkif_t *blkif;
+ 	struct xenbus_watch backend_watch;
+ 	unsigned major;
+ 	unsigned minor;
+ 	char *mode;
+}; 
 
 blkif_t *blkif_alloc(domid_t domid);
 void blkif_free(blkif_t *blkif);
diff -r 360f9dc71f51 -r 55ab88937d77 linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c
--- a/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c	Tue Jun 13 11:41:15 2006 +0100
+++ b/linux-2.6-xen-sparse/drivers/xen/blkback/xenbus.c	Thu Jun 15 13:57:11 2006 +0900
@@ -27,16 +27,6 @@
 #define DPRINTK(fmt, args...)				\
 	pr_debug("blkback/xenbus (%s:%d) " fmt ".\n",	\
 		 __FUNCTION__, __LINE__, ##args)
-
-struct backend_info
-{
-	struct xenbus_device *dev;
-	blkif_t *blkif;
-	struct xenbus_watch backend_watch;
-	unsigned major;
-	unsigned minor;
-	char *mode;
-};
 
 static void connect(struct backend_info *);
 static int connect_ring(struct backend_info *);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[NAHieu]

On 6/21/06, Satoshi Uchida <s-uchida@ap.jp.nec.com> wrote:
>
> This patch add /xen/proc/vbd.
> This proc file stores counts of I/O requests for VBD.
> I/O requests is counted for each Domains.
>
> For example, in case of four VMs, outputs of reading /xen/proc/vbd are as follows.
>
> root@dom0 ~]# more /proc/xen/vbd
> Domain 4 - oo        0  rd    18699  wr        2
>  xvd 4  8:21 - oo        0  rd    18699  wr        2
> Domain 7 - oo        0  rd      869  wr      376
>  xvd 7  8:22 - oo        0  rd      869  wr      376
> Domain 8 - oo        0  rd      721  wr      197
>  xvd 8  8:23 - oo        0  rd      721  wr      197
> Domain 9 - oo        0  rd      640  wr       77
>  xvd 9  8:24 - oo        0  rd      640  wr       77
>
> The outputs show the following information.
>
> The first line shows domain information.
>      Detail: Domain [Domain ID] - oo [oo-reqs] rd  [rd-reqs] wr [wr-reqs].

Cool! But what is the "oo" and "oo-reqs" in the above line?

Thanks.
H

RE: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Satoshi Uchida]

[-- Attachment #1.1: Type: text/plain, Size: 637 bytes --]

Hi, 

>>
>> The first line shows domain information.
>>      Detail: Domain [Domain ID] - oo [oo-reqs] rd  [rd-reqs] wr
[wr-reqs].
>
>Cool! But what is the "oo" and "oo-reqs" in the above line?

The oo-reqs is counted up in the case that request is blocked for no pending
queues in blkback.
BUt, I don't know that what does OO stand for.

If oo-req counts up huge amount in short time, so it means that I/O request
is very high load.
Namely, I/O performance of devices will not do I/O requests of VMs(DomUs).

If oo-req is not increased, so it means that I/O performance is enough for
I/O reuests of VMs(DomUs).


Thanks,
Satoshi UCHIDA

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4036 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[Keir Fraser]

On 21 Jun 2006, at 10:51, Satoshi Uchida wrote:

>
> This patch add /xen/proc/vbd.
> This proc file stores counts of I/O requests for VBD.
> I/O requests is counted for each Domains.

We keep getting complained at for adding stuff to /proc. Shouldn't this 
go under the blkback's sysfs directory somewhere?

  -- Keir

Re: [PATCH 1/3] Output Virtual Block Device requests information to /proc/xen/vbd

[NAHieu]

Hi,

I looked at the code, and it seems "oo" stands of "out of", which
indicates how much we were out of request (Xen supports up to 64
requests in queue)

Few minor comments:
- The variable dom_io_info_wq is unused, should be removed.
- printk("%s: out of memory\n", __FUNCTION__) should be used with KERN_ERR

I have one question: in your below output:

> Domain 4 - oo        0  rd    18699  wr        2
>  xvd 4  8:21 - oo        0  rd    18699  wr        2

I notice that you output the domid with xvd (4), but in the above
line, we already had that information (Domain 4). So why you need to
show the domid again?

Thanks,
H