On Wed, Mar 23 2005, Mark Seger wrote:
>
> >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.
Sounds very strange, it is generic code so should work for all.
Different storage?
> 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?
The reason that the sector counting is done in end_that_request_first()
is that it may not be valid in end_that_request_last().
end_that_request_first() may be invoked several times for a single
request, so I did not move the 'number of io count' there as well as
that would require additional tracking in the request. But
end_that_request_last() will in 99.9% of the cases be called _right_
after end_that_request_first(), so I think it should work fine. The
cases where that doesn't happen is for partial io completions.
--
Jens Axboe
-
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]
