BSD accounting IO stats

PATCH In review Guillaume Thouvenin 3 participants 4 messages Aug 2, 2004

Subsystems (5)

VFS Block layer MM core Scheduler +1 more subsystems

lore

Share & tools

lore

Guillaume Thouvenin

author

2004-08-02 11:36 UTC #0 lore raw

PATCH 1/1 BSD accounting IO stats

Hello,

Here is a small patch for 2.6.8-rc2 that updates information about
bytes and blocks read and written. Fields are already in the BSD accounting files but they are never updated. The patch is based on the CSA accounting patch and it doesn't interfere with Tim Schmielau changes (BSD process accounting version 3).

Modifications are:
- It adds four counters in the task_struct (chars read, chars written, blocks read and blocks written). I think it's interesting to separate read and write even if this difference is not made in the BSD accounting.

- Those fields are updated in:
fs/read_write.c for bytes
[in sys_read(), sys_readv(), sys_write() and sys_writev]
drivers/block/ll_rw_blk.c for blocks
[in drive_stat_acct()]

Hope This Help
Guillaume

drivers/block/ll_rw_blk.c +2 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/drivers/block/ll_rw_blk.c linux-2.6.8-rc2+BSDacct_IO/drivers/block/ll_rw_blk.c
@@ -1949,10 +1949,12 @@ void drive_stat_acct(struct request *rq,
 
 	if (rw == READ) {
 		disk_stat_add(rq->rq_disk, read_sectors, nr_sectors);
+		current->rblk += nr_sectors;
 		if (!new_io)
 			disk_stat_inc(rq->rq_disk, read_merges);
 	} else if (rw == WRITE) {
 		disk_stat_add(rq->rq_disk, write_sectors, nr_sectors);
+		current->wblk += nr_sectors;
 		if (!new_io)
 			disk_stat_inc(rq->rq_disk, write_merges);
 	}

fs/read_write.c +12 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/fs/read_write.c linux-2.6.8-rc2+BSDacct_IO/fs/read_write.c
@@ -279,6 +279,9 @@ asmlinkage ssize_t sys_read(unsigned int
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->rchar++;
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sys_read);

@@ -295,6 +298,9 @@ asmlinkage ssize_t sys_write(unsigned in
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->wchar++;
+
 	return ret;
 }

@@ -517,6 +523,9 @@ sys_readv(unsigned long fd, const struct
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->rchar++;
+
 	return ret;
 }

@@ -533,6 +542,9 @@ sys_writev(unsigned long fd, const struc
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->wchar++;
+
 	return ret;
 }

include/linux/sched.h +2 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/include/linux/sched.h linux-2.6.8-rc2+BSDacct_IO/include/linux/sched.h
@@ -465,6 +465,8 @@ struct task_struct {
 	char comm[16];
 /* file system info */
 	int link_count, total_link_count;
+/* I/O info: chars read/written, blocks read/written */
+	unsigned long rchar, wchar, rblk, wblk;
 /* ipc stuff */
 	struct sysv_sem sysvsem;
 /* CPU-specific state of this task */

kernel/acct.c +2 -2

diff -uprN -X dontdiff linux-2.6.8-rc2/kernel/acct.c linux-2.6.8-rc2+BSDacct_IO/kernel/acct.c
@@ -465,8 +465,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(current->rblk + current->wblk);
 	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);

kernel/fork.c +1 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/kernel/fork.c linux-2.6.8-rc2+BSDacct_IO/kernel/fork.c
@@ -960,6 +960,7 @@ struct task_struct *copy_process(unsigne
 
 	p->utime = p->stime = 0;
 	p->cutime = p->cstime = 0;
+	p->rchar = p->wchar = p->rblk = p->wblk = 0;
 	p->lock_depth = -1;		/* -1 = no lock */
 	p->start_time = get_jiffies_64();
 	p->security = NULL;

Rik van Riel

2004-08-02 12:14 UTC #1 lore raw

Quoted 5 lines · Guillaume Thouvenin

Modifications are:
- It adds four counters in the task_struct (chars read, chars written, blocks read and blocks written). I think it's interesting to separate read and write even if this difference is not made in the BSD accounting.

Nice. One question though:

Review asmlinkage ssize_t sys_write(unsigned in · line 305

Quoted 9 lines

(quoted diff) +3 -0

@@ -295,6 +298,9 @@ asmlinkage ssize_t sys_write(unsigned in
                fput_light(file, fput_needed);
        }
 
+       if (ret > 0)
+               current->wchar++;
+
        return ret;
 }

Shouldn't that be "current->wchar += ret" ?

Signature · 3 lines

"Debugging is twice as hard as writing the code in the first place.
Therefore, if you write the code as cleverly as possible, you are,
by definition, not smart enough to debug it." - Brian W. Kernighan

drivers/block/ll_rw_blk.c +2 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/drivers/block/ll_rw_blk.c linux-2.6.8-rc2+BSDacct_IO/drivers/block/ll_rw_blk.c
@@ -1949,10 +1949,12 @@ void drive_stat_acct(struct request *rq,
 
 	if (rw == READ) {
 		disk_stat_add(rq->rq_disk, read_sectors, nr_sectors);
+		current->rblk += nr_sectors;
 		if (!new_io)
 			disk_stat_inc(rq->rq_disk, read_merges);
 	} else if (rw == WRITE) {
 		disk_stat_add(rq->rq_disk, write_sectors, nr_sectors);
+		current->wblk += nr_sectors;
 		if (!new_io)
 			disk_stat_inc(rq->rq_disk, write_merges);
 	}

fs/read_write.c +12 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/fs/read_write.c linux-2.6.8-rc2+BSDacct_IO/fs/read_write.c
@@ -279,6 +279,9 @@ asmlinkage ssize_t sys_read(unsigned int
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->rchar++;
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(sys_read);

@@ -295,6 +298,9 @@ asmlinkage ssize_t sys_write(unsigned in
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->wchar++;
+
 	return ret;
 }

@@ -517,6 +523,9 @@ sys_readv(unsigned long fd, const struct
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->rchar++;
+
 	return ret;
 }

@@ -533,6 +542,9 @@ sys_writev(unsigned long fd, const struc
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->wchar++;
+
 	return ret;
 }

include/linux/sched.h +2 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/include/linux/sched.h linux-2.6.8-rc2+BSDacct_IO/include/linux/sched.h
@@ -465,6 +465,8 @@ struct task_struct {
 	char comm[16];
 /* file system info */
 	int link_count, total_link_count;
+/* I/O info: chars read/written, blocks read/written */
+	unsigned long rchar, wchar, rblk, wblk;
 /* ipc stuff */
 	struct sysv_sem sysvsem;
 /* CPU-specific state of this task */

kernel/acct.c +2 -2

diff -uprN -X dontdiff linux-2.6.8-rc2/kernel/acct.c linux-2.6.8-rc2+BSDacct_IO/kernel/acct.c
@@ -465,8 +465,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(current->rblk + current->wblk);
 	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);

kernel/fork.c +1 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/kernel/fork.c linux-2.6.8-rc2+BSDacct_IO/kernel/fork.c
@@ -960,6 +960,7 @@ struct task_struct *copy_process(unsigne
 
 	p->utime = p->stime = 0;
 	p->cutime = p->cstime = 0;
+	p->rchar = p->wchar = p->rblk = p->wblk = 0;
 	p->lock_depth = -1;		/* -1 = no lock */
 	p->start_time = get_jiffies_64();
 	p->security = NULL;

Guillaume Thouvenin

author

2004-08-02 12:58 UTC #2 lore raw

Quoted 12 lines · Rik van Riel

Quoted 6 lines · Guillaume Thouvenin
Modifications are:
- It adds four counters in the task_struct (chars read, chars written, blocks read and blocks written). I think it's
interesting to
separate read and write even if this difference is not made in the BSD accounting.
Nice. One question though:

Quoted 10 lines

Review asmlinkage ssize_t sys_write(unsigned in · line 304

Quoted 8 lines

(quoted diff) +2 -0

@@ -295,6 +298,9 @@ asmlinkage ssize_t sys_write(unsigned in
                fput_light(file, fput_needed);
        }
                                                    +       if (ret > 0)
+               current->wchar++;
+
        return ret;
 }

   Shouldn't that be   "current->wchar += ret"  ?

Yes you're right. I attach a new patch with those modifications. Thank you

Guillaume

drivers/block/ll_rw_blk.c +2 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/drivers/block/ll_rw_blk.c linux-2.6.8-rc2+BSDacct_IO/drivers/block/ll_rw_blk.c
@@ -1949,10 +1949,12 @@ void drive_stat_acct(struct request *rq,
 
 	if (rw == READ) {
 		disk_stat_add(rq->rq_disk, read_sectors, nr_sectors);
+		current->rblk += nr_sectors;
 		if (!new_io)
 			disk_stat_inc(rq->rq_disk, read_merges);
 	} else if (rw == WRITE) {
 		disk_stat_add(rq->rq_disk, write_sectors, nr_sectors);
+		current->wblk += nr_sectors;
 		if (!new_io)
 			disk_stat_inc(rq->rq_disk, write_merges);
 	}

fs/read_write.c +17 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/fs/read_write.c linux-2.6.8-rc2+BSDacct_IO/fs/read_write.c
@@ -279,6 +279,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);

@@ -295,6 +298,9 @@ asmlinkage ssize_t sys_write(unsigned in
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->wchar += ret;
+
 	return ret;
 }

@@ -517,6 +523,9 @@ sys_readv(unsigned long fd, const struct
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->rchar += ret;
+
 	return ret;
 }

@@ -533,6 +542,9 @@ sys_writev(unsigned long fd, const struc
 		fput_light(file, fput_needed);
 	}
 
+	if (ret > 0)
+		current->wchar += ret;
+
 	return ret;
 }

@@ -607,6 +619,11 @@ static ssize_t do_sendfile(int out_fd, i
 
 	retval = in_file->f_op->sendfile(in_file, ppos, count, file_send_actor, out_file);
 
+	if (retval > 0) {
+		current->rchar += retval;
+		current->wchar += retval;
+	}
+	
 	if (*ppos > max)
 		retval = -EOVERFLOW;

include/linux/sched.h +2 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/include/linux/sched.h linux-2.6.8-rc2+BSDacct_IO/include/linux/sched.h
@@ -465,6 +465,8 @@ struct task_struct {
 	char comm[16];
 /* file system info */
 	int link_count, total_link_count;
+/* I/O info: chars read/written, blocks read/written */
+	unsigned long rchar, wchar, rblk, wblk;
 /* ipc stuff */
 	struct sysv_sem sysvsem;
 /* CPU-specific state of this task */

kernel/acct.c +2 -2

diff -uprN -X dontdiff linux-2.6.8-rc2/kernel/acct.c linux-2.6.8-rc2+BSDacct_IO/kernel/acct.c
@@ -465,8 +465,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(current->rblk + current->wblk);
 	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);

kernel/fork.c +1 -0

diff -uprN -X dontdiff linux-2.6.8-rc2/kernel/fork.c linux-2.6.8-rc2+BSDacct_IO/kernel/fork.c
@@ -960,6 +960,7 @@ struct task_struct *copy_process(unsigne
 
 	p->utime = p->stime = 0;
 	p->cutime = p->cstime = 0;
+	p->rchar = p->wchar = p->rblk = p->wblk = 0;
 	p->lock_depth = -1;		/* -1 = no lock */
 	p->start_time = get_jiffies_64();
 	p->security = NULL;

Andrew Morton

2004-08-02 16:59 UTC #3 lore raw

In reply to Guillaume Thouvenin

Quoted 6 lines · Guillaume Thouvenin

diff -uprN -X dontdiff linux-2.6.8-rc2/drivers/block/ll_rw_blk.c linux-2.6.8-rc2+BSDacct_IO/drivers/block/ll_rw_blk.c
--- linux-2.6.8-rc2/drivers/block/ll_rw_blk.c 2004-07-18 06:57:42.000000000 +0200
+++ linux-2.6.8-rc2+BSDacct_IO/drivers/block/ll_rw_blk.c 2004-07-27 09:17:33.149321480 +0200
@@ -1949,10 +1949,12 @@ void drive_stat_acct(struct request *rq,

Quoted 11 lines

if (rw == READ) {
disk_stat_add(rq->rq_disk, read_sectors, nr_sectors);
+ current->rblk += nr_sectors;
if (!new_io)
disk_stat_inc(rq->rq_disk, read_merges);
} else if (rw == WRITE) {
disk_stat_add(rq->rq_disk, write_sectors, nr_sectors);
+ current->wblk += nr_sectors;
if (!new_io)
disk_stat_inc(rq->rq_disk, write_merges);
}

The `wblk' accounting will be quite wrong - most writes will be accounted to pdflush, kjournald, etc.