From 20e5c81fcff89535dced2ed71cf24c6c648ff40e Mon Sep 17 00:00:00 2001 From: "Chen, Kenneth W" Date: Thu, 13 Oct 2005 21:48:42 +0200 Subject: [patch] remove gendisk->stamp_idle field struct gendisk has these two fields: stamp, stamp_idle. Update to stamp_idle is always in sync with stamp and they are always the same. Therefore, it does not add any value in having two fields tracking same timestamp. Suggest to remove it. Also, we should only update gendisk stats with non-zero value. Advantage is that we don't have to needlessly calculate memory address, and then add zero to the content. Signed-off-by: Ken Chen Signed-off-by: Jens Axboe --- drivers/block/ll_rw_blk.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/ll_rw_blk.c b/drivers/block/ll_rw_blk.c index baedac5..c42071f 100644 --- a/drivers/block/ll_rw_blk.c +++ b/drivers/block/ll_rw_blk.c @@ -2433,13 +2433,12 @@ void disk_round_stats(struct gendisk *disk) { unsigned long now = jiffies; - __disk_stat_add(disk, time_in_queue, - disk->in_flight * (now - disk->stamp)); + if (disk->in_flight) { + __disk_stat_add(disk, time_in_queue, + disk->in_flight * (now - disk->stamp)); + __disk_stat_add(disk, io_ticks, (now - disk->stamp)); + } disk->stamp = now; - - if (disk->in_flight) - __disk_stat_add(disk, io_ticks, (now - disk->stamp_idle)); - disk->stamp_idle = now; } /* -- cgit v1.1 From b2982649ce38293b14684b26bcda20cfc54164e6 Mon Sep 17 00:00:00 2001 From: "Chen, Kenneth W" Date: Thu, 13 Oct 2005 21:49:29 +0200 Subject: 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 Signed-off-by: Jens Axboe --- drivers/block/ll_rw_blk.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/block') diff --git a/drivers/block/ll_rw_blk.c b/drivers/block/ll_rw_blk.c index c42071f..4e2b1b0 100644 --- a/drivers/block/ll_rw_blk.c +++ b/drivers/block/ll_rw_blk.c @@ -2433,6 +2433,9 @@ void disk_round_stats(struct gendisk *disk) { 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)); -- cgit v1.1 From 2824bc9328467127083c1325f54b67d298c333b2 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Thu, 20 Oct 2005 10:56:41 +0200 Subject: [PATCH] fix try_module_get race in elevator_find This patch removes try_module_get race in elevator_find. try_module_get should always be called with the spinlock protecting what the module init/cleanup routines register/unregister to held. In the case of elevators, we should be holding elv_list to avoid it going away between spin_unlock_irq and try_module_get. Signed-off-by: Tejun Heo Signed-off-by: Jens Axboe --- drivers/block/elevator.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'drivers/block') diff --git a/drivers/block/elevator.c b/drivers/block/elevator.c index 98f0126..4144f30 100644 --- a/drivers/block/elevator.c +++ b/drivers/block/elevator.c @@ -97,7 +97,6 @@ static struct elevator_type *elevator_find(const char *name) struct elevator_type *e = NULL; struct list_head *entry; - spin_lock_irq(&elv_list_lock); list_for_each(entry, &elv_list) { struct elevator_type *__e; @@ -108,7 +107,6 @@ static struct elevator_type *elevator_find(const char *name) break; } } - spin_unlock_irq(&elv_list_lock); return e; } @@ -120,12 +118,15 @@ static void elevator_put(struct elevator_type *e) static struct elevator_type *elevator_get(const char *name) { - struct elevator_type *e = elevator_find(name); + struct elevator_type *e; - if (!e) - return NULL; - if (!try_module_get(e->elevator_owner)) - return NULL; + spin_lock_irq(&elv_list_lock); + + e = elevator_find(name); + if (e && !try_module_get(e->elevator_owner)) + e = NULL; + + spin_unlock_irq(&elv_list_lock); return e; } @@ -153,11 +154,15 @@ static char chosen_elevator[16]; static void elevator_setup_default(void) { + struct elevator_type *e; + /* * check if default is set and exists */ - if (chosen_elevator[0] && elevator_find(chosen_elevator)) + if (chosen_elevator[0] && (e = elevator_get(chosen_elevator))) { + elevator_put(e); return; + } #if defined(CONFIG_IOSCHED_AS) strcpy(chosen_elevator, "anticipatory"); @@ -555,10 +560,9 @@ void elv_unregister_queue(struct request_queue *q) int elv_register(struct elevator_type *e) { + spin_lock_irq(&elv_list_lock); if (elevator_find(e->elevator_name)) BUG(); - - spin_lock_irq(&elv_list_lock); list_add_tail(&e->list, &elv_list); spin_unlock_irq(&elv_list_lock); -- cgit v1.1