[PATCH] Fix for disk statistics omissions in 2.2 and 2.3

Malcolm Beattie (mbeattie@sable.ox.ac.uk)
Tue, 26 Oct 1999 11:08:14 +0100 (BST)


Malcolm Beattie writes:
> While working on a block device hot migration utility, I've come
> across what looks like a problem in the way block requests are
> handled/merged/counted/something in ll_rw_blk.c.

I've found that it's a bug in make_request(): it doesn't update the
block statistics in kstat.dk_drive when it merges a request into the
current one. Here's a patch against 2.2.12 which works for me (cat
/proc/stat to see that it now does count disk_rblk and disk_wblk
correctly instead of missing out about 126 of every 128 sectors).
I'd guess it's suitable for 2.2.14, please Alan?

Following that is the same patch made against 2.3.23 (I made a
separate one since a one character whitespace change in drive_stat_acct
means you'd have to use "patch -l" to ignore whitespace otherwise).
Please can that be applied, Linus?

----------------------- statsbug-2.2.12.patch -----------------------
--- drivers/block/ll_rw_blk.c.statsbug Tue Oct 26 10:20:26 1999
+++ drivers/block/ll_rw_blk.c Tue Oct 26 10:19:20 1999
@@ -277,15 +277,39 @@
else ro_bits[major][minor >> 5] &= ~(1 << (minor & 31));
}

-static inline void drive_stat_acct(int cmd, unsigned long nr_sectors,
- short disk_index)
+static inline void drive_stat_acct(struct request *req,
+ unsigned long nr_sectors, int new_io)
{
- kstat.dk_drive[disk_index]++;
- if (cmd == READ) {
- kstat.dk_drive_rio[disk_index]++;
+ int major = MAJOR(req->rq_dev);
+ int minor = MINOR(req->rq_dev);
+ unsigned int disk_index;
+
+ switch (major) {
+ case DAC960_MAJOR+0:
+ disk_index = (minor & 0x00f8) >> 3;
+ break;
+ case SCSI_DISK0_MAJOR:
+ disk_index = (minor & 0x00f0) >> 4;
+ break;
+ case IDE0_MAJOR: /* same as HD_MAJOR */
+ case XT_DISK_MAJOR:
+ disk_index = (minor & 0x0040) >> 6;
+ break;
+ case IDE1_MAJOR:
+ disk_index = ((minor & 0x0040) >> 6) + 2;
+ break;
+ default:
+ return;
+ }
+ if (disk_index >= DK_NDRIVE)
+ return;
+
+ kstat.dk_drive[disk_index] += new_io;
+ if (req->cmd == READ) {
+ kstat.dk_drive_rio[disk_index] += new_io;
kstat.dk_drive_rblk[disk_index] += nr_sectors;
- } else if (cmd == WRITE) {
- kstat.dk_drive_wio[disk_index]++;
+ } else if (req->cmd == WRITE) {
+ kstat.dk_drive_wio[disk_index] += new_io;
kstat.dk_drive_wblk[disk_index] += nr_sectors;
} else
printk(KERN_ERR "drive_stat_acct: cmd not R/W?\n");
@@ -304,35 +328,11 @@
void add_request(struct blk_dev_struct * dev, struct request * req)
{
int major = MAJOR(req->rq_dev);
- int minor = MINOR(req->rq_dev);
struct request * tmp, **current_request;
- short disk_index;
unsigned long flags;
int queue_new_request = 0;

- switch (major) {
- case DAC960_MAJOR+0:
- disk_index = (minor & 0x00f8) >> 3;
- if (disk_index < 4)
- drive_stat_acct(req->cmd, req->nr_sectors, disk_index);
- break;
- case SCSI_DISK0_MAJOR:
- disk_index = (minor & 0x00f0) >> 4;
- if (disk_index < 4)
- drive_stat_acct(req->cmd, req->nr_sectors, disk_index);
- break;
- case IDE0_MAJOR: /* same as HD_MAJOR */
- case XT_DISK_MAJOR:
- disk_index = (minor & 0x0040) >> 6;
- drive_stat_acct(req->cmd, req->nr_sectors, disk_index);
- break;
- case IDE1_MAJOR:
- disk_index = ((minor & 0x0040) >> 6) + 2;
- drive_stat_acct(req->cmd, req->nr_sectors, disk_index);
- default:
- break;
- }
-
+ drive_stat_acct(req, req->nr_sectors, 1);
req->next = NULL;

/*
@@ -573,6 +573,7 @@
req->bhtail->b_reqnext = bh;
req->bhtail = bh;
req->nr_sectors += count;
+ drive_stat_acct(req, count, 0);
/* Can we now merge this req with the next? */
attempt_merge(req, max_sectors, max_segments);
/* or to the beginning? */
@@ -589,6 +590,7 @@
req->current_nr_sectors = count;
req->sector = sector;
req->nr_sectors += count;
+ drive_stat_acct(req, count, 0);
} else
continue;

----------------------- statsbug-2.2.12.patch -----------------------

----------------------- statsbug-2.3.23.patch -----------------------
--- linux-2.3.23/drivers/block/ll_rw_blk.c.ORI Tue Oct 26 10:55:58 1999
+++ linux-2.3.23/drivers/block/ll_rw_blk.c Tue Oct 26 11:00:10 1999
@@ -286,15 +286,39 @@
else ro_bits[major][minor >> 5] &= ~(1 << (minor & 31));
}

-static inline void drive_stat_acct(int cmd, unsigned long nr_sectors,
- short disk_index)
+static inline void drive_stat_acct(struct request *req,
+ unsigned long nr_sectors, int new_io)
{
- kstat.dk_drive[disk_index]++;
- if (cmd == READ) {
- kstat.dk_drive_rio[disk_index]++;
+ int major = MAJOR(req->rq_dev);
+ int minor = MINOR(req->rq_dev);
+ unsigned int disk_index;
+
+ switch (major) {
+ case DAC960_MAJOR+0:
+ disk_index = (minor & 0x00f8) >> 3;
+ break;
+ case SCSI_DISK0_MAJOR:
+ disk_index = (minor & 0x00f0) >> 4;
+ break;
+ case IDE0_MAJOR: /* same as HD_MAJOR */
+ case XT_DISK_MAJOR:
+ disk_index = (minor & 0x0040) >> 6;
+ break;
+ case IDE1_MAJOR:
+ disk_index = ((minor & 0x0040) >> 6) + 2;
+ break;
+ default:
+ return;
+ }
+ if (disk_index >= DK_NDRIVE)
+ return;
+
+ kstat.dk_drive[disk_index] += new_io;
+ if (req->cmd == READ) {
+ kstat.dk_drive_rio[disk_index] += new_io;
kstat.dk_drive_rblk[disk_index] += nr_sectors;
- } else if (cmd == WRITE) {
- kstat.dk_drive_wio[disk_index]++;
+ } else if (req->cmd == WRITE) {
+ kstat.dk_drive_wio[disk_index] += new_io;
kstat.dk_drive_wblk[disk_index] += nr_sectors;
} else
printk(KERN_ERR "drive_stat_acct: cmd not R/W?\n");
@@ -313,35 +337,11 @@
void add_request(struct blk_dev_struct * dev, struct request * req)
{
int major = MAJOR(req->rq_dev);
- int minor = MINOR(req->rq_dev);
struct request * tmp, **current_request;
- short disk_index;
unsigned long flags;
int queue_new_request = 0;

- switch (major) {
- case DAC960_MAJOR+0:
- disk_index = (minor & 0x00f8) >> 3;
- if (disk_index < 4)
- drive_stat_acct(req->cmd, req->nr_sectors, disk_index);
- break;
- case SCSI_DISK0_MAJOR:
- disk_index = (minor & 0x00f0) >> 4;
- if (disk_index < 4)
- drive_stat_acct(req->cmd, req->nr_sectors, disk_index);
- break;
- case IDE0_MAJOR: /* same as HD_MAJOR */
- case XT_DISK_MAJOR:
- disk_index = (minor & 0x0040) >> 6;
- drive_stat_acct(req->cmd, req->nr_sectors, disk_index);
- break;
- case IDE1_MAJOR:
- disk_index = ((minor & 0x0040) >> 6) + 2;
- drive_stat_acct(req->cmd, req->nr_sectors, disk_index);
- default:
- break;
- }
-
+ drive_stat_acct(req, req->nr_sectors, 1);
req->next = NULL;

/*
@@ -575,6 +575,7 @@
req->bhtail->b_reqnext = bh;
req->bhtail = bh;
req->nr_sectors += count;
+ drive_stat_acct(req, count, 0);
/* Can we now merge this req with the next? */
attempt_merge(req, max_sectors);
/* or to the beginning? */
@@ -585,6 +586,7 @@
req->current_nr_sectors = count;
req->sector = sector;
req->nr_sectors += count;
+ drive_stat_acct(req, count, 0);
} else
continue;

----------------------- statsbug-2.3.23.patch -----------------------

--Malcolm

-- 
Malcolm Beattie <mbeattie@sable.ox.ac.uk>
Unix Systems Programmer
Oxford University Computing Services

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/