I don't like this patch, it adds 4 * sizeof(unsigned long) to struct
request when it can be solved without adding anything. The idea is
sound, though, the current way the stats are done isn't very
interesting.
Actually I wasn't all that excited about using the extra variable
myself. However, I wasn't entirely sure what was going on and this at
least allowed me to test the concept without doing anything harmful.
How about accounting merges the way we currently do it, since that piece
of the stats _is_ interesting at queueing time. And then account
completion in __end_that_request_first(). Untested patch attached.
I also agree with your suggestion about keeping the merged counts where
they are and am copying the author of iostat to suggest the man page be
updated to reflect the fact that merges are counts for requests queued
rather than 'issued to the device' as it currently states.
re: your patch - I did try it on both an Operton and Xeon box. It
worked find on the Opeteron and reported 0 for all the sectors on the
Xeon. If nothing immediately jumps to your mind could it have been
something I did wrong? I'll try another build after I send this along,
but I don't see how that will help as I did the first one from a brand
new source kit.
The one thing that still jumps out at me about this patch is that the
sectors are being counted in one routine and the number of I/Os in
another. If the best place to update the sector counts is indeed where
you suggest doing it, is there any reason not to move the update code
for all the disk stats from end_that_request_last() to that same place
as well for consistency and for better assurances that they are updated
as close to the same point in time as possible?
-mark
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]