[PATCH]: adding counters to count bytes read/written

[PATCH]: adding counters to count bytes read/written

[Manik Raina]

Hi Linus,

	This patch adds 2 counters to the task_struct for
	counting how many bytes were read/written using
	the read()/write() system calls. 

	These counters may be useful in determining how
	many IO requests are made by each process.


diff -u -r ../temp/linux-2.5.12/fs/proc/array.c ./fs/proc/array.c
--- ../temp/linux-2.5.12/fs/proc/array.c	Wed May  1 05:38:45 2002
+++ ./fs/proc/array.c	Mon May 20 09:18:16 2002
@@ -346,7 +346,7 @@
 	read_unlock(&tasklist_lock);
 	res = sprintf(buffer,"%d (%s) %c %d %d %d %d %d %lu %lu \
 %lu %lu %lu %lu %lu %ld %ld %ld %ld %ld %ld %lu %lu %ld %lu %lu %lu %lu %lu \
-%lu %lu %lu %lu %lu %lu %lu %lu %d %d\n",
+%lu %lu %lu %lu %lu %lu %lu %lu %d %d %d %d\n",
 		task->pid,
 		task->comm,
 		state,
@@ -388,8 +388,10 @@
 		wchan,
 		task->nswap,
 		task->cnswap,
-		task->exit_signal,
-		task->thread_info->cpu);
+ 	        task->exit_signal,
+		task->thread_info->cpu,
+		task->bytes_read,
+		task->bytes_written);
 	if(mm)
 		mmput(mm);
 	return res;
diff -u -r ../temp/linux-2.5.12/fs/read_write.c ./fs/read_write.c
--- ../temp/linux-2.5.12/fs/read_write.c	Wed May  1 05:38:59 2002
+++ ./fs/read_write.c	Mon May 20 09:48:30 2002
@@ -180,8 +180,10 @@
 					ret = read(file, buf, count, &file->f_pos);
 			}
 		}
-		if (ret > 0)
+		if (ret > 0) {
 			dnotify_parent(file->f_dentry, DN_ACCESS);
+			current->bytes_read += ret;
+		}
 		fput(file);
 	}
 	return ret;
@@ -206,8 +208,10 @@
 					ret = write(file, buf, count, &file->f_pos);
 			}
 		}
-		if (ret > 0)
+		if (ret > 0) {
 			dnotify_parent(file->f_dentry, DN_MODIFY);
+			current->bytes_written += ret;
+		}
 		fput(file);
 	}
 	return ret;
diff -u -r ../temp/linux-2.5.12/include/linux/sched.h ./include/linux/sched.h
--- ../temp/linux-2.5.12/include/linux/sched.h	Wed May  1 05:38:47 2002
+++ ./include/linux/sched.h	Mon May 20 09:25:32 2002
@@ -315,6 +315,7 @@
 	int link_count, total_link_count;
 	struct tty_struct *tty; /* NULL if no tty */
 	unsigned int locks; /* How many file locks are being held */
+	unsigned int bytes_written, bytes_read;
 /* ipc stuff */
 	struct sysv_sem sysvsem;
 /* CPU-specific state of this task */

Re: [PATCH]: adding counters to count bytes read/written

[Matti Aarnio]

On Mon, May 20, 2002 at 03:09:36PM +0530, Manik Raina wrote:
> Hi Linus,
> 
> 	This patch adds 2 counters to the task_struct for
> 	counting how many bytes were read/written using
> 	the read()/write() system calls. 
> 
> 	These counters may be useful in determining how
> 	many IO requests are made by each process.

  These are defined as UINTegers, are you sure that is appropriate type ?
  What to do when they will overflow ?  For short term activity tracking 
  they may be ok (4GB/200 MB/sec = 20 sec to wrap around), but for accounting
  the overflow might not be liked thing..

  For short-term IO-activity tracking they may indeed make sense, but I
  would add another pair of counters to assist on that tracking.  Namely
  "values at the end of previous interval", which are maintained by the
  activity tracking code.

  Reading one byte at the time won't grow those counters very fast, but will
  cause massive amounts of syscalls, and context switches, so tracking data
  amount alone isn't good enough.

....
> diff -u -r ../temp/linux-2.5.12/include/linux/sched.h ./include/linux/sched.h
> --- ../temp/linux-2.5.12/include/linux/sched.h	Wed May  1 05:38:47 2002
> +++ ./include/linux/sched.h	Mon May 20 09:25:32 2002
> @@ -315,6 +315,7 @@
>  	int link_count, total_link_count;
>  	struct tty_struct *tty; /* NULL if no tty */
>  	unsigned int locks; /* How many file locks are being held */
> +	unsigned int bytes_written, bytes_read;
>  /* ipc stuff */
>  	struct sysv_sem sysvsem;
>  /* CPU-specific state of this task */

/Matti Aarnio

Re: [PATCH]: adding counters to count bytes read/written

[Manik Raina]

	Thanks for the comments Matti, Please see inline ...

Matti Aarnio wrote:
> 
> On Mon, May 20, 2002 at 03:09:36PM +0530, Manik Raina wrote:
> > Hi Linus,
> >
> >       This patch adds 2 counters to the task_struct for
> >       counting how many bytes were read/written using
> >       the read()/write() system calls.
> >
> >       These counters may be useful in determining how
> >       many IO requests are made by each process.
> 
>   These are defined as UINTegers, are you sure that is appropriate type ?
>   What to do when they will overflow ?  For short term activity tracking
>   they may be ok (4GB/200 MB/sec = 20 sec to wrap around), but for accounting
>   the overflow might not be liked thing..


	How about 64 bit counters ? i feel those should go on without
	wraparound for a _very_ long time.

	Did you have anything else in mind ?
	
> 
>   For short-term IO-activity tracking they may indeed make sense, but I
>   would add another pair of counters to assist on that tracking.  Namely
>   "values at the end of previous interval", which are maintained by the
>   activity tracking code.

	Would this still be required if the counters are 64 bit ?

> 
>   Reading one byte at the time won't grow those counters very fast, but will
>   cause massive amounts of syscalls, and context switches, so tracking data
>   amount alone isn't good enough.

	What else would you suggest i track ?
	thanks
	Manik

> 
> ....
> > diff -u -r ../temp/linux-2.5.12/include/linux/sched.h ./include/linux/sched.h
> > --- ../temp/linux-2.5.12/include/linux/sched.h        Wed May  1 05:38:47 2002
> > +++ ./include/linux/sched.h   Mon May 20 09:25:32 2002
> > @@ -315,6 +315,7 @@
> >       int link_count, total_link_count;
> >       struct tty_struct *tty; /* NULL if no tty */
> >       unsigned int locks; /* How many file locks are being held */
> > +     unsigned int bytes_written, bytes_read;
> >  /* ipc stuff */
> >       struct sysv_sem sysvsem;
> >  /* CPU-specific state of this task */
> 
> /Matti Aarnio

Re: [PATCH]: adding counters to count bytes read/written

[Mike Fedyk]

On Mon, May 20, 2002 at 04:33:40PM +0530, Manik Raina wrote:
> 
> 	Thanks for the comments Matti, Please see inline ...
> 
> Matti Aarnio wrote:
> > 
> > On Mon, May 20, 2002 at 03:09:36PM +0530, Manik Raina wrote:
> > > Hi Linus,
> > >
> > >       This patch adds 2 counters to the task_struct for
> > >       counting how many bytes were read/written using
> > >       the read()/write() system calls.
> > >
> > >       These counters may be useful in determining how
> > >       many IO requests are made by each process.
> > 
> >   These are defined as UINTegers, are you sure that is appropriate type ?
> >   What to do when they will overflow ?  For short term activity tracking
> >   they may be ok (4GB/200 MB/sec = 20 sec to wrap around), but for accounting
> >   the overflow might not be liked thing..
> 
> 
> 	How about 64 bit counters ? i feel those should go on without
> 	wraparound for a _very_ long time.

I really doubt that 64bit counters in a fast path will be accepted.  You'll
really need to justify something like that.

Have you looked at the process accounting code?

Re: [PATCH]: adding counters to count bytes read/written

[Matti Aarnio]

On Mon, May 20, 2002 at 04:33:40PM +0530, Manik Raina wrote:
> 	Thanks for the comments Matti, Please see inline ...
> 
> Matti Aarnio wrote:
> > On Mon, May 20, 2002 at 03:09:36PM +0530, Manik Raina wrote:
> > > Hi Linus,
> > >       This patch adds 2 counters to the task_struct for
> > >       counting how many bytes were read/written using
> > >       the read()/write() system calls.
> > >
> > >       These counters may be useful in determining how
> > >       many IO requests are made by each process.
> > 
> >   These are defined as UINTegers, are you sure that is appropriate type ?
> >   What to do when they will overflow ?  For short term activity tracking
> >   they may be ok (4GB/200 MB/sec = 20 sec to wrap around), but for accounting
> >   the overflow might not be liked thing..
> 
> 	How about 64 bit counters ? i feel those should go on without
> 	wraparound for a _very_ long time.
> 
> 	Did you have anything else in mind ?

    Considering how few registers ix86 architecture has, and
    them being 32 bit wide ones, regular (once a second ?)
    task scanning and detecting such overflows would be most
    usefull approach.  Less code, and register pressure in
    the fast path.

> >   For short-term IO-activity tracking they may indeed make sense, but I
> >   would add another pair of counters to assist on that tracking.  Namely
> >   "values at the end of previous interval", which are maintained by the
> >   activity tracking code.
> 
> 	Would this still be required if the counters are 64 bit ?

    I don't think so.   E.g. even with 1 GB/sec bandwidth (hmm..
    perhaps that is achievable in next few years..) and 32 bit
    counters it is just a matter of how frequently you scan these
    variables.  To know how much data has been moved since the
    last measurement, you need to keep the value of the previous
    scan time around.  E.g.:

        int intervalread = NN->bytes_read - NN->previous_bytes_read;
        NN->previous_bytes_read = NN->bytes_read;

    Add there code to detect when "bytes_read" has overflowed, and
    external monitoring code can then combine those components into
    64 bit quantity..  (The /proc/ printout code can do it)

> >   Reading one byte at the time won't grow those counters very fast, but will
> >   cause massive amounts of syscalls, and context switches, so tracking data
> >   amount alone isn't good enough.
> 
> 	What else would you suggest i track ?
> 	thanks
> 	Manik

   Number of syscalls,  The more your process makes syscalls, the more
   it makes context switches, but still does not consume its time quanta.
   (Consider: 100 clocks for syscall, 2.5 GHz CPU -> 25 M syscalls/sec ?)

   Use of these counters should be defined.  Mere "it would be nice to
   collect data" is not very good motivation for Linus to include the 
   code.  (Of course I would like to see all BSD facility accounting
   things when a process exits -- knowing number of X/Y/Z might sometimes
   tell me something usefull..)

   Defining some believable theory/algorithm to use these variales
   in e.g. scheduling goodness calculation..  perhaps in a form of
   "scan all processes 10 times a second for IO/syscall activity
    tuner for scheduler"

   Will the analysis truly need real-time calculation in the scheduler's
   fast-path, or can it be based on one computed scalar value which is
   updated one to ten times a second ?


/Matti Aarnio