ide-io.c, kernel 2.4.22 Fix for IO stats in /proc/partitions, was Re: sard/iostat disk I/O statistics/accounting for 2.5.8-pre3

I found the cause of ide disks' ios_in_flight going negative in /proc/partitions.

It's due to unbalanced calls to up_ios and down_ios. After an explicit drive command, ide-io.c's ide_end_drive_cmd calls end_that_request_last which eventually calls down_ios to decrement ios_in_flight, however there is no corresponding call to up_ios when the command is initiated.

My guess is that ios_in_flight goes negative when the drive is idle because many people run hdparm in an init script, and this decrements ios_in_flight early on. It stays off center from there.

The following hack to ide_end_drive_cmd is a workaround, I would rather call up_ios appropriately, so that explicit ide commands are properly accounted. However I'm having a hard time identifying all the places that initiate a low-level drive command. I'll look into a proper fix, but someone else probably knows the ide layer better than me.

Chad

@@ -148,6 +148,7 @@
 	ide_hwif_t *hwif = HWIF(drive);
 	unsigned long flags;
 	struct request *rq;
+	struct completion *waiting;
 
 	spin_lock_irqsave(&io_request_lock, flags);
 	rq = HWGROUP(drive)->rq;

@@ -221,7 +222,13 @@
 	spin_lock_irqsave(&io_request_lock, flags);
 	blkdev_dequeue_request(rq);
 	HWGROUP(drive)->rq = NULL;
-	end_that_request_last(rq);
+
+	waiting = req->waiting;
+	req_finished_io(req);
+	blkdev_release_request(req);
+	if (waiting)
+		complete(waiting);
+
 	spin_unlock_irqrestore(&io_request_lock, flags);
 }
 

[...]

J.A. Magallon <[email protected]>      \                 Software is like sex:
werewolf.able.es                         \           It's better when it's free
Mandrake Linux release 9.2 (Cooker) for i586
Linux 2.4.23-pre4-jam1 (gcc 3.3.1 (Mandrake Linux 9.2 3.3.1-2mdk))

[broken patch elided...]

Good catch. Not that patch, obviously. :) I don't know where I came up with that one; even with the req -> rq change it includes the call to req_finished_io, so it's exactly a no-op.

Here's the one that I compiled and tested. It makes /proc/partitions report correctly.

Thanks,
Chad

@@ -148,6 +148,7 @@
 	ide_hwif_t *hwif = HWIF(drive);
 	unsigned long flags;
 	struct request *rq;
+	struct completion *waiting;
 
 	spin_lock_irqsave(&io_request_lock, flags);
 	rq = HWGROUP(drive)->rq;

@@ -221,7 +222,12 @@
 	spin_lock_irqsave(&io_request_lock, flags);
 	blkdev_dequeue_request(rq);
 	HWGROUP(drive)->rq = NULL;
-	end_that_request_last(rq);
+
+	waiting = rq->waiting;
+	blkdev_release_request(rq);
+	if (waiting)
+		complete(waiting);
+
 	spin_unlock_irqrestore(&io_request_lock, flags);
 }