Re: [PATCH 2.6.9-rc2 1/2] enhanced I/O accounting data collection

Re: [PATCH 2.6.9-rc2 1/2] enhanced I/O accounting data collection

[Jens Axboe]

Hi,

> Index: linux/drivers/block/ll_rw_blk.c
> ===================================================================
> --- linux.orig/drivers/block/ll_rw_blk.c	2004-09-12 22:31:31.000000000 -0700
> +++ linux/drivers/block/ll_rw_blk.c	2004-09-27 12:37:04.374234677 -0700
> @@ -1741,6 +1741,7 @@
> {
> 	DEFINE_WAIT(wait);
> 	struct request *rq;
>+	unsigned long start_wait = jiffies;
> 
> 	generic_unplug_device(q);
> 	do {
>@@ -1769,6 +1770,7 @@
> 		finish_wait(&rl->wait[rw], &wait);
> 	} while (!rq);
> 
>+	current->bwtime += (unsigned long) jiffies - start_wait;
> 	return rq;
> }

What is the purpose of this hunk alone as block io accounting? It
doesn't make any sense to me - you are accounting the time a process
spends sleeping on a congested queue, it has nothing to do with the
bandwidth time it used. Which, btw, isn't so easy to account on queueing
hardware.

Just curious on what you are trying to achieve here.
 
-- 
Jens Axboe

Re: [PATCH 2.6.9-rc2 1/2] enhanced I/O accounting data collection

[Jay Lan]

You are right, Jens. In our earlier posting, we also included block
device read/write counters. The block read/write counts are not very
accurate but it fits our customers' needs since they used that
information sort of for performance analysis than for accounting
purpose.

Thus the block read/write counters were removed from our patch so
that we can concentrate on the accounting needs. This bwtime (block
wait time) should have been pulled together with block read/write
counters.

Regards,
  - jay


Jens Axboe wrote:
> Hi,
> 
> 
>>Index: linux/drivers/block/ll_rw_blk.c
>>===================================================================
>>--- linux.orig/drivers/block/ll_rw_blk.c	2004-09-12 22:31:31.000000000 -0700
>>+++ linux/drivers/block/ll_rw_blk.c	2004-09-27 12:37:04.374234677 -0700
>>@@ -1741,6 +1741,7 @@
>>{
>>	DEFINE_WAIT(wait);
>>	struct request *rq;
>>+	unsigned long start_wait = jiffies;
>>
>>	generic_unplug_device(q);
>>	do {
>>@@ -1769,6 +1770,7 @@
>>		finish_wait(&rl->wait[rw], &wait);
>>	} while (!rq);
>>
>>+	current->bwtime += (unsigned long) jiffies - start_wait;
>>	return rq;
>>}
> 
> 
> What is the purpose of this hunk alone as block io accounting? It
> doesn't make any sense to me - you are accounting the time a process
> spends sleeping on a congested queue, it has nothing to do with the
> bandwidth time it used. Which, btw, isn't so easy to account on queueing
> hardware.
> 
> Just curious on what you are trying to achieve here.
>  

[PATCH 2.6.9-rc2 0/2] enhanced accounting data collection

[Jay Lan]

This is an effort of providing an enhanced accounting data collection.

It is intended to offer common data collection method for various
accounting packages including BSD accouting, ELSA, CSA, and any other
acct packages that favor a common layer of data collection, separated
from data presentation layer and management of process groups layer.

This patchset consists of two parts: acct_io and acct_mm as we
identified useful spots for improved data collection in the area
of IO and MM.

This patchset is to replace the previously submitted CSA patchset
of four. The CSA kernel module is a standalone module. The csa_eop
patch was to provide a hook for end-of-process handling and that
can be considered separately unless there is enough common interest.

Now that the patchset is down to IO and MM, i hope it is more
appealing :)

Comments?

Best Regards,
  - jay
---
Jay Lan - Linux System Software
Silicon Graphics Inc., Mountain View, CA

Re: [PATCH 2.6.9-rc2 1/2] enhanced I/O accounting data collection

[Jay Lan]

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

1/2: acct_io

Enhanced I/O accounting data collection.

Signed-off-by: Jay Lan <jlan@sgi.com>


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

Index: linux/drivers/block/ll_rw_blk.c
===================================================================
--- linux.orig/drivers/block/ll_rw_blk.c	2004-09-12 22:31:31.000000000 -0700
+++ linux/drivers/block/ll_rw_blk.c	2004-09-27 12:37:04.374234677 -0700
@@ -1741,6 +1741,7 @@
 {
 	DEFINE_WAIT(wait);
 	struct request *rq;
+	unsigned long start_wait = jiffies;
 
 	generic_unplug_device(q);
 	do {
@@ -1769,6 +1770,7 @@
 		finish_wait(&rl->wait[rw], &wait);
 	} while (!rq);
 
+	current->bwtime += (unsigned long) jiffies - start_wait;
 	return rq;
 }
 
Index: linux/fs/read_write.c
===================================================================
--- linux.orig/fs/read_write.c	2004-09-12 22:32:55.000000000 -0700
+++ linux/fs/read_write.c	2004-09-27 12:37:04.381070659 -0700
@@ -216,8 +216,11 @@
 				ret = file->f_op->read(file, buf, count, pos);
 			else
 				ret = do_sync_read(file, buf, count, pos);
-			if (ret > 0)
+			if (ret > 0) {
 				dnotify_parent(file->f_dentry, DN_ACCESS);
+				current->rchar += ret;
+			}
+			current->syscr++;
 		}
 	}
 
@@ -260,8 +263,11 @@
 				ret = file->f_op->write(file, buf, count, pos);
 			else
 				ret = do_sync_write(file, buf, count, pos);
-			if (ret > 0)
+			if (ret > 0) {
 				dnotify_parent(file->f_dentry, DN_MODIFY);
+				current->wchar += ret;
+			}
+			current->syscw++;
 		}
 	}
 
@@ -540,6 +546,10 @@
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0) {
+		current->rchar += ret;
+	}
+	current->syscr++;
 	return ret;
 }
 
@@ -558,6 +568,10 @@
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0) {
+		current->wchar += ret;
+	}
+	current->syscw++;
 	return ret;
 }
 
@@ -636,6 +650,13 @@
 
 	retval = in_file->f_op->sendfile(in_file, ppos, count, file_send_actor, out_file);
 
+	if (retval > 0) {
+		current->rchar += retval;
+		current->wchar += retval;
+	}
+	current->syscr++;
+	current->syscw++;
+
 	if (*ppos > max)
 		retval = -EOVERFLOW;
 
Index: linux/include/linux/sched.h
===================================================================
--- linux.orig/include/linux/sched.h	2004-09-27 11:57:40.220967100 -0700
+++ linux/include/linux/sched.h	2004-09-27 12:52:51.305237393 -0700
@@ -591,6 +591,9 @@
 	struct rw_semaphore pagg_sem;
 #endif
 
+/* i/o counters(bytes read/written, #syscalls, waittime */
+	unsigned long rchar, wchar, syscr, syscw, bwtime;
+
 };
 
 static inline pid_t process_group(struct task_struct *tsk)