[patch] optimize disk_round_stats

[patch] optimize disk_round_stats

[Chen, Kenneth W]

Following the same idea, it occurs to me that we should only update
disk stat when "now" is different from disk->stamp.  Otherwise, we
are again needlessly adding zero to the stats.


Signed-off-by: Ken Chen <kenneth.w.chen@intel.com>

--- ./drivers/block/ll_rw_blk.c.orig	2005-10-13 11:54:07.474907379 -0700
+++ ./drivers/block/ll_rw_blk.c	2005-10-13 11:54:39.074516367 -0700
@@ -2433,6 +2433,9 @@ void disk_round_stats(struct gendisk *di
 {
 	unsigned long now = jiffies;
 
+	if (now == disk->stamp)
+		return;
+
 	if (disk->in_flight) {
 		__disk_stat_add(disk, time_in_queue,
 				disk->in_flight * (now - disk->stamp));

Re: [patch] optimize disk_round_stats

[Jens Axboe]

On Thu, Oct 13 2005, Chen, Kenneth W wrote:
> Following the same idea, it occurs to me that we should only update
> disk stat when "now" is different from disk->stamp.  Otherwise, we
> are again needlessly adding zero to the stats.

Thanks, also applied.

-- 
Jens Axboe

Re: [patch] optimize disk_round_stats

[Arjan van de Ven]

On Thu, 2005-10-13 at 12:19 -0700, Chen, Kenneth W wrote:
> Following the same idea, it occurs to me that we should only update
> disk stat when "now" is different from disk->stamp.  Otherwise, we
> are again needlessly adding zero to the stats.

have you measured this?
Conditionals in code are not free, so it might well be more expensive...

RE: [patch] optimize disk_round_stats

[Chen, Kenneth W]

Arjan van de Ven wrote on Friday, October 14, 2005 3:20 AM
> On Thu, 2005-10-13 at 12:19 -0700, Chen, Kenneth W wrote:
> > Following the same idea, it occurs to me that we should only update
> > disk stat when "now" is different from disk->stamp.  Otherwise, we
> > are again needlessly adding zero to the stats.
> 
> have you measured this?
> Conditionals in code are not free, so it might well be more expensive...

Yes I did, on a null block driver[1], this optimization gets about
2% improvement.  The reasoning is that for example, user submits
100,000 I/O per second, we only need to update the stats per jiffies.
With latest kernel have 250 Hz as default.  It's 250 updates versus
100,000 updates (with 99,750 updates of adding zero).  I see that the
condition is well worth it.  The address calculation for per cpu disk
stats has lots of indirection and index calculation.  Compiler generates
lots of code just for address calculation.

- Ken


[1] this driver completes an I/O request as soon as it enters block queue.