lkml.org 
[lkml]   [2005]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Patch for inconsistent recording of block device statistics

>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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-04-06 13:30    [W:0.099 / U:2.404 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site