From 48cae885d5a896030588978f503c73c5ed5e62b1 Mon Sep 17 00:00:00 2001 From: Stefan Weinhuber Date: Wed, 11 Feb 2009 10:37:31 +0100 Subject: [S390] dasd: fix race in dasd timer handling In dasd_device_set_timer and dasd_block_set_timer we interpret the return value of mod_timer in a wrong way. If the timer expires in the small window between our check of timer_pending and the call to mod_timer, then the timer will be set, mod_timer returns zero and we will call add_timer for a timer that is already pending. As del_timer and mod_timer do all the necessary checking themselves, we can simplify our code and remove the race a the same time. Signed-off-by: Stefan Weinhuber Signed-off-by: Martin Schwidefsky --- drivers/s390/block/dasd.c | 46 ++++++++++++++++------------------------------ 1 file changed, 16 insertions(+), 30 deletions(-) (limited to 'drivers/s390') diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c index bd5914994142..08c23a921012 100644 --- a/drivers/s390/block/dasd.c +++ b/drivers/s390/block/dasd.c @@ -57,6 +57,8 @@ static void dasd_device_tasklet(struct dasd_device *); static void dasd_block_tasklet(struct dasd_block *); static void do_kick_device(struct work_struct *); static void dasd_return_cqr_cb(struct dasd_ccw_req *, void *); +static void dasd_device_timeout(unsigned long); +static void dasd_block_timeout(unsigned long); /* * SECTION: Operations on the device structure. @@ -99,6 +101,8 @@ struct dasd_device *dasd_alloc_device(void) (unsigned long) device); INIT_LIST_HEAD(&device->ccw_queue); init_timer(&device->timer); + device->timer.function = dasd_device_timeout; + device->timer.data = (unsigned long) device; INIT_WORK(&device->kick_work, do_kick_device); device->state = DASD_STATE_NEW; device->target = DASD_STATE_NEW; @@ -138,6 +142,8 @@ struct dasd_block *dasd_alloc_block(void) INIT_LIST_HEAD(&block->ccw_queue); spin_lock_init(&block->queue_lock); init_timer(&block->timer); + block->timer.function = dasd_block_timeout; + block->timer.data = (unsigned long) block; return block; } @@ -915,19 +921,10 @@ static void dasd_device_timeout(unsigned long ptr) */ void dasd_device_set_timer(struct dasd_device *device, int expires) { - if (expires == 0) { - if (timer_pending(&device->timer)) - del_timer(&device->timer); - return; - } - if (timer_pending(&device->timer)) { - if (mod_timer(&device->timer, jiffies + expires)) - return; - } - device->timer.function = dasd_device_timeout; - device->timer.data = (unsigned long) device; - device->timer.expires = jiffies + expires; - add_timer(&device->timer); + if (expires == 0) + del_timer(&device->timer); + else + mod_timer(&device->timer, jiffies + expires); } /* @@ -935,8 +932,7 @@ void dasd_device_set_timer(struct dasd_device *device, int expires) */ void dasd_device_clear_timer(struct dasd_device *device) { - if (timer_pending(&device->timer)) - del_timer(&device->timer); + del_timer(&device->timer); } static void dasd_handle_killed_request(struct ccw_device *cdev, @@ -1586,19 +1582,10 @@ static void dasd_block_timeout(unsigned long ptr) */ void dasd_block_set_timer(struct dasd_block *block, int expires) { - if (expires == 0) { - if (timer_pending(&block->timer)) - del_timer(&block->timer); - return; - } - if (timer_pending(&block->timer)) { - if (mod_timer(&block->timer, jiffies + expires)) - return; - } - block->timer.function = dasd_block_timeout; - block->timer.data = (unsigned long) block; - block->timer.expires = jiffies + expires; - add_timer(&block->timer); + if (expires == 0) + del_timer(&block->timer); + else + mod_timer(&block->timer, jiffies + expires); } /* @@ -1606,8 +1593,7 @@ void dasd_block_set_timer(struct dasd_block *block, int expires) */ void dasd_block_clear_timer(struct dasd_block *block) { - if (timer_pending(&block->timer)) - del_timer(&block->timer); + del_timer(&block->timer); } /* -- cgit v1.2.3