[patch 2.6.8.1] BSD accounting: update chars transferred value

[patch 2.6.8.1] BSD accounting: update chars transferred value

[Guillaume Thouvenin]

Hello,

  The goal of this patch is to improve BSD accounting by using what 
is done in the CSA into BSD accounting. The final goal is to have a
uniform accounting structure. 

  This patch updates information given by BSD accounting concerning
bytes read and written. A field is already present in the BSD accounting
structure but it is never updated. We don't add information about blocks
read and written because, as it was discussed in previous email, the 
information is inaccurate. Most writes which are flushed delayed would 
get accounted to pdflushd. Thus, one solution to get this kind of 
information is to add counters when the write back is performed. The 
problem is that we don't know how to get information about the IID at 
the page level (ie from struct page). So we remove this information for 
the moment and it will be provided in another patch.

Changelog:
  
  - Adds two counters in the task_struct (rchar, wchar)
  - Init fields during the creation of the process (during the fork)
  - File I/O operations are done through sys_read(), sys_write(), 
    sys_readv(), sys_writev() and sys_sendfile(). Thus we increment 
    counters into those functions except with sys_sendfile(). For the
    latter, the incrementation is done in the do_sendfile() because this
    routine be directly called from the return of sys_sendfile().


Signed-off-by: Guillaume Thouvenin <guillaume.thouvenin@bull.net>


diff -uprN -X dontdiff linux-2.6.8.1-vanilla/fs/read_write.c linux-2.6.8.1-char_IO_acct/fs/read_write.c
--- linux-2.6.8.1-vanilla/fs/read_write.c	2004-08-14 12:55:35.000000000 +0200
+++ linux-2.6.8.1-char_IO_acct/fs/read_write.c	2004-09-08 08:32:29.043438000 +0200
@@ -294,6 +294,9 @@ asmlinkage ssize_t sys_read(unsigned int
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->rchar += ret;
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sys_read);
@@ -312,6 +315,9 @@ asmlinkage ssize_t sys_write(unsigned in
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->wchar += ret;
+
 	return ret;
 }
 
@@ -540,6 +546,9 @@ sys_readv(unsigned long fd, const struct
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->rchar += ret;
+
 	return ret;
 }
 
@@ -558,6 +567,9 @@ sys_writev(unsigned long fd, const struc
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->wchar += ret;
+
 	return ret;
 }
 
@@ -639,6 +651,11 @@ static ssize_t do_sendfile(int out_fd, i
 	if (*ppos > max)
 		retval = -EOVERFLOW;
 
+	if (retval > 0) {
+		current->rchar += retval;
+		current->wchar += retval;
+	}
+
 fput_out:
 	fput_light(out_file, fput_needed_out);
 fput_in:
diff -uprN -X dontdiff linux-2.6.8.1-vanilla/include/linux/sched.h linux-2.6.8.1-char_IO_acct/include/linux/sched.h
--- linux-2.6.8.1-vanilla/include/linux/sched.h	2004-08-14 12:54:49.000000000 +0200
+++ linux-2.6.8.1-char_IO_acct/include/linux/sched.h	2004-09-08 08:08:47.565535488 +0200
@@ -523,6 +523,9 @@ struct task_struct {
 	unsigned long ptrace_message;
 	siginfo_t *last_siginfo; /* For ptrace use.  */
 
+/* IO counters: bytes read, bytes written */
+	unsigned long rchar, wchar;
+
 #ifdef CONFIG_NUMA
   	struct mempolicy *mempolicy;
   	short il_next;		/* could be shared with used_math */
diff -uprN -X dontdiff linux-2.6.8.1-vanilla/kernel/acct.c linux-2.6.8.1-char_IO_acct/kernel/acct.c
--- linux-2.6.8.1-vanilla/kernel/acct.c	2004-08-14 12:55:33.000000000 +0200
+++ linux-2.6.8.1-char_IO_acct/kernel/acct.c	2004-09-08 08:10:04.758800328 +0200
@@ -464,8 +464,8 @@ static void do_acct_process(long exitcod
 	}
 	vsize = vsize / 1024;
 	ac.ac_mem = encode_comp_t(vsize);
-	ac.ac_io = encode_comp_t(0 /* current->io_usage */);	/* %% */
-	ac.ac_rw = encode_comp_t(ac.ac_io / 1024);
+	ac.ac_io = encode_comp_t(current->rchar + current->wchar);
+	ac.ac_rw = encode_comp_t(0 /* ac.ac_io / 1024 */);	/* %% */
 	ac.ac_minflt = encode_comp_t(current->min_flt);
 	ac.ac_majflt = encode_comp_t(current->maj_flt);
 	ac.ac_swaps = encode_comp_t(0);
diff -uprN -X dontdiff linux-2.6.8.1-vanilla/kernel/fork.c linux-2.6.8.1-char_IO_acct/kernel/fork.c
--- linux-2.6.8.1-vanilla/kernel/fork.c	2004-08-14 12:54:49.000000000 +0200
+++ linux-2.6.8.1-char_IO_acct/kernel/fork.c	2004-09-08 08:11:24.308706904 +0200
@@ -965,6 +965,7 @@ struct task_struct *copy_process(unsigne
 	p->security = NULL;
 	p->io_context = NULL;
 	p->audit_context = NULL;
+	p->rchar = p->wchar = 0;
 #ifdef CONFIG_NUMA
  	p->mempolicy = mpol_copy(p->mempolicy);
  	if (IS_ERR(p->mempolicy)) {

Re: [patch 2.6.8.1] BSD accounting: update chars transferred value

[Christoph Hellwig]

On Wed, Sep 08, 2004 at 11:06:57AM +0200, Guillaume Thouvenin wrote:
> Hello,
> 
>   The goal of this patch is to improve BSD accounting by using what 
> is done in the CSA into BSD accounting. The final goal is to have a
> uniform accounting structure. 
> 
>   This patch updates information given by BSD accounting concerning
> bytes read and written. A field is already present in the BSD accounting
> structure but it is never updated. We don't add information about blocks
> read and written because, as it was discussed in previous email, the 
> information is inaccurate. Most writes which are flushed delayed would 
> get accounted to pdflushd. Thus, one solution to get this kind of 
> information is to add counters when the write back is performed. The 
> problem is that we don't know how to get information about the IID at 
> the page level (ie from struct page). So we remove this information for 
> the moment and it will be provided in another patch.
> 
> Changelog:
>   
>   - Adds two counters in the task_struct (rchar, wchar)
>   - Init fields during the creation of the process (during the fork)
>   - File I/O operations are done through sys_read(), sys_write(), 
>     sys_readv(), sys_writev() and sys_sendfile(). Thus we increment 
>     counters into those functions except with sys_sendfile(). For the
>     latter, the incrementation is done in the do_sendfile() because this
>     routine be directly called from the return of sys_sendfile().

I think it should be done in vfs_readv/vfs_write.  Else we'll miss the
updates from inekrnel consumers like nfsd.

Re: [patch 2.6.8.1] BSD accounting: update chars transferred value

[Guillaume Thouvenin]

On Wed, Sep 08, 2004 at 10:17:04AM +0100, Christoph Hellwig wrote:
> On Wed, Sep 08, 2004 at 11:06:57AM +0200, Guillaume Thouvenin wrote:
> > Hello,
> > 
> >   The goal of this patch is to improve BSD accounting by using what 
> > is done in the CSA into BSD accounting. The final goal is to have a
> > uniform accounting structure. 
> > 
> >   This patch updates information given by BSD accounting concerning
> > bytes read and written. A field is already present in the BSD accounting
> > structure but it is never updated. We don't add information about blocks
> > read and written because, as it was discussed in previous email, the 
> > information is inaccurate. Most writes which are flushed delayed would 
> > get accounted to pdflushd. Thus, one solution to get this kind of 
> > information is to add counters when the write back is performed. The 
> > problem is that we don't know how to get information about the IID at 
> > the page level (ie from struct page). So we remove this information for 
> > the moment and it will be provided in another patch.
> > 
> > Changelog:
> >   
> >   - Adds two counters in the task_struct (rchar, wchar)
> >   - Init fields during the creation of the process (during the fork)
> >   - File I/O operations are done through sys_read(), sys_write(), 
> >     sys_readv(), sys_writev() and sys_sendfile(). Thus we increment 
> >     counters into those functions except with sys_sendfile(). For the
> >     latter, the incrementation is done in the do_sendfile() because this
> >     routine be directly called from the return of sys_sendfile().
> 
> I think it should be done in vfs_readv/vfs_write.  Else we'll miss the
> updates from inekrnel consumers like nfsd.

Yes you're right. vfs_readv() and vfs_writev() called do_readv_writev().
Thus I will send a new patch which updates counters in functions
vfs_read(), vfs_write(), do_readv_writev() and sys_sendfile()

[patch 2.6.8.1] BSD accounting: update chars transferred value

[Guillaume Thouvenin]

Hello,

  The goal of this patch is to improve BSD accounting by using what
is done in the CSA into BSD accounting. The final goal is to have a
uniform accounting structure.

  This patch updates information given by BSD accounting concerning
bytes read and written. A field is already present in the BSD accounting
structure but it is never updated. We don't add information about blocks
read and written because, as it was discussed in previous email, the
information is inaccurate. Most writes which are flushed delayed would
get accounted to pdflushd. Thus, one solution to get this kind of
information is to add counters when the write back is performed. The
problem is that we don't know how to get information about the PID at
the page level (ie from struct page). So we remove this information for
the moment and it will be provided in another patch.

Changelog:

  - Adds two counters in the task_struct (rchar, wchar)
  - Init fields during the creation of the process (during the fork)
  - File I/O operations are done through sys_read(), sys_write(),
    sys_readv(), sys_writev() and sys_sendfile(). Some file system, like 
    NFS, are using vfs_readv() and if we don't add counters here, we
    will miss something. sys_read() and sys_write() used respectively
    vfs_read() and vfs_write(). Routines sys_readv() and sys_writev()
    are using vfs_readv() and vfs_writev(). Both functions are calling
    do_readv_writev(). Thus we increment counters in vfs_read(), 
    vfs_write(), do_readv_writev() and do_sendfile(). For the
    latter, the incrementation is done in the do_sendfile() because this
    routine is directly called from the return of sys_sendfile(). 


Signed-off-by: Guillaume Thouvenin <guillaume.thouvenin@bull.net>

diff -uprN -X dontdiff linux-2.6.8.1-vanilla/fs/read_write.c linux-2.6.8.1-char_IO_acct/fs/read_write.c
--- linux-2.6.8.1-vanilla/fs/read_write.c	2004-08-14 12:55:35.000000000 +0200
+++ linux-2.6.8.1-char_IO_acct/fs/read_write.c	2004-09-08 13:11:43.832260408 +0200
@@ -216,8 +216,10 @@ ssize_t vfs_read(struct file *file, char
 				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;
+			}
 		}
 	}
 
@@ -260,8 +262,10 @@ ssize_t vfs_write(struct file *file, con
 				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;
+			}
 		}
 	}
 
@@ -496,6 +500,14 @@ out:
 	if ((ret + (type == READ)) > 0)
 		dnotify_parent(file->f_dentry,
 				(type == READ) ? DN_ACCESS : DN_MODIFY);
+
+	if (ret > 0) {
+		if (type == READ)
+			current->rchar += ret;
+		else /* type == WRITE */
+			current->wchar += ret;
+	}
+
 	return ret;
 }
 
@@ -644,6 +656,11 @@ fput_out:
 fput_in:
 	fput_light(in_file, fput_needed_in);
 out:
+	if (retval > 0) {
+		current->rchar += retval;
+		current->wchar += retval;
+	}
+	
 	return retval;
 }
 
diff -uprN -X dontdiff linux-2.6.8.1-vanilla/include/linux/sched.h linux-2.6.8.1-char_IO_acct/include/linux/sched.h
--- linux-2.6.8.1-vanilla/include/linux/sched.h	2004-08-14 12:54:49.000000000 +0200
+++ linux-2.6.8.1-char_IO_acct/include/linux/sched.h	2004-09-08 12:49:35.634177208 +0200
@@ -523,6 +523,9 @@ struct task_struct {
 	unsigned long ptrace_message;
 	siginfo_t *last_siginfo; /* For ptrace use.  */
 
+/* IO counters: bytes read, bytes written */
+	unsigned long rchar, wchar;
+
 #ifdef CONFIG_NUMA
   	struct mempolicy *mempolicy;
   	short il_next;		/* could be shared with used_math */
diff -uprN -X dontdiff linux-2.6.8.1-vanilla/kernel/acct.c linux-2.6.8.1-char_IO_acct/kernel/acct.c
--- linux-2.6.8.1-vanilla/kernel/acct.c	2004-08-14 12:55:33.000000000 +0200
+++ linux-2.6.8.1-char_IO_acct/kernel/acct.c	2004-09-08 12:50:50.058862936 +0200
@@ -464,8 +464,8 @@ static void do_acct_process(long exitcod
 	}
 	vsize = vsize / 1024;
 	ac.ac_mem = encode_comp_t(vsize);
-	ac.ac_io = encode_comp_t(0 /* current->io_usage */);	/* %% */
-	ac.ac_rw = encode_comp_t(ac.ac_io / 1024);
+	ac.ac_io = encode_comp_t(current->rchar + current->wchar);
+	ac.ac_rw = encode_comp_t(0 /* ac.ac_io / 1024 */);
 	ac.ac_minflt = encode_comp_t(current->min_flt);
 	ac.ac_majflt = encode_comp_t(current->maj_flt);
 	ac.ac_swaps = encode_comp_t(0);
diff -uprN -X dontdiff linux-2.6.8.1-vanilla/kernel/fork.c linux-2.6.8.1-char_IO_acct/kernel/fork.c
--- linux-2.6.8.1-vanilla/kernel/fork.c	2004-08-14 12:54:49.000000000 +0200
+++ linux-2.6.8.1-char_IO_acct/kernel/fork.c	2004-09-08 12:50:07.931267304 +0200
@@ -965,6 +965,7 @@ struct task_struct *copy_process(unsigne
 	p->security = NULL;
 	p->io_context = NULL;
 	p->audit_context = NULL;
+	p->rchar = p->wchar = 0;
 #ifdef CONFIG_NUMA
  	p->mempolicy = mpol_copy(p->mempolicy);
  	if (IS_ERR(p->mempolicy)) {

Re: [patch 2.6.8.1] BSD accounting: update chars transferred value

[Jay Lan]

Hi Guillaume,

This patch is a subset of csa_io with your patch deals with character
IO only.

I can see that merge csa_io's change at vfs_writev() and vfs_readv()
into your change at do_readv_writev(). However, the code change is
not really common code in that a) the operation type is different and
2) the fields to add to are different, so you end up doing extra check 
of file operation type (READ vs WRITE). I would be happy if either
your patch or mine is accepted, but i think it does extra work to put
the changes into the common routine (ie do_readv_writev).

Shall we combine your patch and SGI's csa_io patch?

Andrew, csa_io, csa_mm, and csa_eop patches can be considered
separately. Please consider taking in csa_io since we have another
accounting IO data collection patch in this one. :)

Cheers,
  - jay


Guillaume Thouvenin wrote:
> Hello,
> 
>   The goal of this patch is to improve BSD accounting by using what
> is done in the CSA into BSD accounting. The final goal is to have a
> uniform accounting structure.
> 
>   This patch updates information given by BSD accounting concerning
> bytes read and written. A field is already present in the BSD accounting
> structure but it is never updated. We don't add information about blocks
> read and written because, as it was discussed in previous email, the
> information is inaccurate. Most writes which are flushed delayed would
> get accounted to pdflushd. Thus, one solution to get this kind of
> information is to add counters when the write back is performed. The
> problem is that we don't know how to get information about the PID at
> the page level (ie from struct page). So we remove this information for
> the moment and it will be provided in another patch.
> 
> Changelog:
> 
>   - Adds two counters in the task_struct (rchar, wchar)
>   - Init fields during the creation of the process (during the fork)
>   - File I/O operations are done through sys_read(), sys_write(),
>     sys_readv(), sys_writev() and sys_sendfile(). Some file system, like 
>     NFS, are using vfs_readv() and if we don't add counters here, we
>     will miss something. sys_read() and sys_write() used respectively
>     vfs_read() and vfs_write(). Routines sys_readv() and sys_writev()
>     are using vfs_readv() and vfs_writev(). Both functions are calling
>     do_readv_writev(). Thus we increment counters in vfs_read(), 
>     vfs_write(), do_readv_writev() and do_sendfile(). For the
>     latter, the incrementation is done in the do_sendfile() because this
>     routine is directly called from the return of sys_sendfile(). 
> 
> 
> Signed-off-by: Guillaume Thouvenin <guillaume.thouvenin@bull.net>
> 
> diff -uprN -X dontdiff linux-2.6.8.1-vanilla/fs/read_write.c linux-2.6.8.1-char_IO_acct/fs/read_write.c
> --- linux-2.6.8.1-vanilla/fs/read_write.c	2004-08-14 12:55:35.000000000 +0200
> +++ linux-2.6.8.1-char_IO_acct/fs/read_write.c	2004-09-08 13:11:43.832260408 +0200
> @@ -216,8 +216,10 @@ ssize_t vfs_read(struct file *file, char
>  				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;
> +			}
>  		}
>  	}
>  
> @@ -260,8 +262,10 @@ ssize_t vfs_write(struct file *file, con
>  				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;
> +			}
>  		}
>  	}
>  
> @@ -496,6 +500,14 @@ out:
>  	if ((ret + (type == READ)) > 0)
>  		dnotify_parent(file->f_dentry,
>  				(type == READ) ? DN_ACCESS : DN_MODIFY);
> +
> +	if (ret > 0) {
> +		if (type == READ)
> +			current->rchar += ret;
> +		else /* type == WRITE */
> +			current->wchar += ret;
> +	}
> +
>  	return ret;
>  }
>  
> @@ -644,6 +656,11 @@ fput_out:
>  fput_in:
>  	fput_light(in_file, fput_needed_in);
>  out:
> +	if (retval > 0) {
> +		current->rchar += retval;
> +		current->wchar += retval;
> +	}
> +	
>  	return retval;
>  }
>  
> diff -uprN -X dontdiff linux-2.6.8.1-vanilla/include/linux/sched.h linux-2.6.8.1-char_IO_acct/include/linux/sched.h
> --- linux-2.6.8.1-vanilla/include/linux/sched.h	2004-08-14 12:54:49.000000000 +0200
> +++ linux-2.6.8.1-char_IO_acct/include/linux/sched.h	2004-09-08 12:49:35.634177208 +0200
> @@ -523,6 +523,9 @@ struct task_struct {
>  	unsigned long ptrace_message;
>  	siginfo_t *last_siginfo; /* For ptrace use.  */
>  
> +/* IO counters: bytes read, bytes written */
> +	unsigned long rchar, wchar;
> +
>  #ifdef CONFIG_NUMA
>    	struct mempolicy *mempolicy;
>    	short il_next;		/* could be shared with used_math */
> diff -uprN -X dontdiff linux-2.6.8.1-vanilla/kernel/acct.c linux-2.6.8.1-char_IO_acct/kernel/acct.c
> --- linux-2.6.8.1-vanilla/kernel/acct.c	2004-08-14 12:55:33.000000000 +0200
> +++ linux-2.6.8.1-char_IO_acct/kernel/acct.c	2004-09-08 12:50:50.058862936 +0200
> @@ -464,8 +464,8 @@ static void do_acct_process(long exitcod
>  	}
>  	vsize = vsize / 1024;
>  	ac.ac_mem = encode_comp_t(vsize);
> -	ac.ac_io = encode_comp_t(0 /* current->io_usage */);	/* %% */
> -	ac.ac_rw = encode_comp_t(ac.ac_io / 1024);
> +	ac.ac_io = encode_comp_t(current->rchar + current->wchar);
> +	ac.ac_rw = encode_comp_t(0 /* ac.ac_io / 1024 */);
>  	ac.ac_minflt = encode_comp_t(current->min_flt);
>  	ac.ac_majflt = encode_comp_t(current->maj_flt);
>  	ac.ac_swaps = encode_comp_t(0);
> diff -uprN -X dontdiff linux-2.6.8.1-vanilla/kernel/fork.c linux-2.6.8.1-char_IO_acct/kernel/fork.c
> --- linux-2.6.8.1-vanilla/kernel/fork.c	2004-08-14 12:54:49.000000000 +0200
> +++ linux-2.6.8.1-char_IO_acct/kernel/fork.c	2004-09-08 12:50:07.931267304 +0200
> @@ -965,6 +965,7 @@ struct task_struct *copy_process(unsigne
>  	p->security = NULL;
>  	p->io_context = NULL;
>  	p->audit_context = NULL;
> +	p->rchar = p->wchar = 0;
>  #ifdef CONFIG_NUMA
>   	p->mempolicy = mpol_copy(p->mempolicy);
>   	if (IS_ERR(p->mempolicy)) {

Re: [patch 2.6.8.1] BSD accounting: update chars transferred value

[Guillaume Thouvenin]

On Fri, Sep 10, 2004 at 04:10:56PM -0700, Jay Lan wrote:
> This patch is a subset of csa_io with your patch deals with character
> IO only.

Yes you are right. This patch only deals with character IO because I 
don't know yet how to get information for blocks IO. As I said my goal 
is to provide a good solution for accounting. BSD-accounting is already 
in the kernel and CSA provide more metrics so I think it could be good 
to add some CSA accounting values in the BSD-accounting. 

> I can see that merge csa_io's change at vfs_writev() and vfs_readv()
> into your change at do_readv_writev(). However, the code change is
> not really common code in that a) the operation type is different and
> 2) the fields to add to are different, so you end up doing extra check 
> of file operation type (READ vs WRITE). I would be happy if either
> your patch or mine is accepted, but i think it does extra work to put
> the changes into the common routine (ie do_readv_writev).

As you notice, vfs_writev() and vfs_readv() both call do_readv_writev()
and as fields to add are different I added a test on the operation type.
I though that it was interesting to put a changes in the common routine
but you are right that it has a cost (the file operation check). As the 
changes can be done in vfs_readv() and vfs_writev() instead of one single 
routine (do_readv_writev()) I though this choice was good but if the
extra cost is a problem I agree with your solution. Thank you to point
this out.

> Shall we combine your patch and SGI's csa_io patch?

IMHO, it could be very interesting to combine your patch and mine.
BSD-accounting is doing per-process accounting and CSA also doing
per-process accounting even if the goal is a per-job accounting. Thus, I
think that it can be good to combine both. It isn't necessary to have
two different accounting systems in the kernel. 

Is it difficult for you to add what you are doing with CSA in the
BSD-accounting file? Maybe the solution is to remove BSD-accounting in
favor of CSA accounting? Personally, I don't care if we keep
BSD-accounting or if we remove it to add CSA accounting.

Best,
Guillaume

Re: [patch 2.6.8.1] BSD accounting: update chars transferred value

[Jay Lan]

Guillaume Thouvenin wrote:
> On Fri, Sep 10, 2004 at 04:10:56PM -0700, Jay Lan wrote:
> 
>>This patch is a subset of csa_io with your patch deals with character
>>IO only.
> 
> 
> Yes you are right. This patch only deals with character IO because I 
> don't know yet how to get information for blocks IO. As I said my goal 
> is to provide a good solution for accounting. BSD-accounting is already 
> in the kernel and CSA provide more metrics so I think it could be good 
> to add some CSA accounting values in the BSD-accounting. 

Agreed!

> 
> 
>>I can see that merge csa_io's change at vfs_writev() and vfs_readv()
>>into your change at do_readv_writev(). However, the code change is
>>not really common code in that a) the operation type is different and
>>2) the fields to add to are different, so you end up doing extra check 
>>of file operation type (READ vs WRITE). I would be happy if either
>>your patch or mine is accepted, but i think it does extra work to put
>>the changes into the common routine (ie do_readv_writev).
> 
> 
> As you notice, vfs_writev() and vfs_readv() both call do_readv_writev()
> and as fields to add are different I added a test on the operation type.
> I though that it was interesting to put a changes in the common routine
> but you are right that it has a cost (the file operation check). As the 
> changes can be done in vfs_readv() and vfs_writev() instead of one single 
> routine (do_readv_writev()) I though this choice was good but if the
> extra cost is a problem I agree with your solution. Thank you to point
> this out
> 
> 
>>Shall we combine your patch and SGI's csa_io patch?
> 
> 
> IMHO, it could be very interesting to combine your patch and mine.
> BSD-accounting is doing per-process accounting and CSA also doing
> per-process accounting even if the goal is a per-job accounting. Thus, I
> think that it can be good to combine both. It isn't necessary to have
> two different accounting systems in the kernel. 
> 
> Is it difficult for you to add what you are doing with CSA in the
> BSD-accounting file? Maybe the solution is to remove BSD-accounting in
> favor of CSA accounting? Personally, I don't care if we keep
> BSD-accounting or if we remove it to add CSA accounting.

Your patch and SGI's csa_io are about accounting data gathering, so
merging these two patches still agrees with the favored approach: one
common data collection while we allow different data presentation
layer.

We have removed block IO from csa_io patch. The difference between
these two patches are data colleciton regarding to READ/WRITE system
calls, and block IO wait time (per process) that SGI and Cray customers
demanded.

Thanks,
  - jay

> 
> Best,
> Guillaume