diff options
author | Stefan Weinhuber <wein@de.ibm.com> | 2009-02-11 04:37:31 -0500 |
---|---|---|
committer | Martin Schwidefsky <schwidefsky@de.ibm.com> | 2009-02-11 04:37:40 -0500 |
commit | 48cae885d5a896030588978f503c73c5ed5e62b1 (patch) | |
tree | 52668ad63447cd3395181295f8795c580b77d99e /drivers | |
parent | ca0b4b7d2cb57a2e24d7e48ce9b411b9baa3bf63 (diff) |
[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 <wein@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/s390/block/dasd.c | 46 |
1 files changed, 16 insertions, 30 deletions
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 *); | |||
57 | static void dasd_block_tasklet(struct dasd_block *); | 57 | static void dasd_block_tasklet(struct dasd_block *); |
58 | static void do_kick_device(struct work_struct *); | 58 | static void do_kick_device(struct work_struct *); |
59 | static void dasd_return_cqr_cb(struct dasd_ccw_req *, void *); | 59 | static void dasd_return_cqr_cb(struct dasd_ccw_req *, void *); |
60 | static void dasd_device_timeout(unsigned long); | ||
61 | static void dasd_block_timeout(unsigned long); | ||
60 | 62 | ||
61 | /* | 63 | /* |
62 | * SECTION: Operations on the device structure. | 64 | * SECTION: Operations on the device structure. |
@@ -99,6 +101,8 @@ struct dasd_device *dasd_alloc_device(void) | |||
99 | (unsigned long) device); | 101 | (unsigned long) device); |
100 | INIT_LIST_HEAD(&device->ccw_queue); | 102 | INIT_LIST_HEAD(&device->ccw_queue); |
101 | init_timer(&device->timer); | 103 | init_timer(&device->timer); |
104 | device->timer.function = dasd_device_timeout; | ||
105 | device->timer.data = (unsigned long) device; | ||
102 | INIT_WORK(&device->kick_work, do_kick_device); | 106 | INIT_WORK(&device->kick_work, do_kick_device); |
103 | device->state = DASD_STATE_NEW; | 107 | device->state = DASD_STATE_NEW; |
104 | device->target = DASD_STATE_NEW; | 108 | device->target = DASD_STATE_NEW; |
@@ -138,6 +142,8 @@ struct dasd_block *dasd_alloc_block(void) | |||
138 | INIT_LIST_HEAD(&block->ccw_queue); | 142 | INIT_LIST_HEAD(&block->ccw_queue); |
139 | spin_lock_init(&block->queue_lock); | 143 | spin_lock_init(&block->queue_lock); |
140 | init_timer(&block->timer); | 144 | init_timer(&block->timer); |
145 | block->timer.function = dasd_block_timeout; | ||
146 | block->timer.data = (unsigned long) block; | ||
141 | 147 | ||
142 | return block; | 148 | return block; |
143 | } | 149 | } |
@@ -915,19 +921,10 @@ static void dasd_device_timeout(unsigned long ptr) | |||
915 | */ | 921 | */ |
916 | void dasd_device_set_timer(struct dasd_device *device, int expires) | 922 | void dasd_device_set_timer(struct dasd_device *device, int expires) |
917 | { | 923 | { |
918 | if (expires == 0) { | 924 | if (expires == 0) |
919 | if (timer_pending(&device->timer)) | 925 | del_timer(&device->timer); |
920 | del_timer(&device->timer); | 926 | else |
921 | return; | 927 | mod_timer(&device->timer, jiffies + expires); |
922 | } | ||
923 | if (timer_pending(&device->timer)) { | ||
924 | if (mod_timer(&device->timer, jiffies + expires)) | ||
925 | return; | ||
926 | } | ||
927 | device->timer.function = dasd_device_timeout; | ||
928 | device->timer.data = (unsigned long) device; | ||
929 | device->timer.expires = jiffies + expires; | ||
930 | add_timer(&device->timer); | ||
931 | } | 928 | } |
932 | 929 | ||
933 | /* | 930 | /* |
@@ -935,8 +932,7 @@ void dasd_device_set_timer(struct dasd_device *device, int expires) | |||
935 | */ | 932 | */ |
936 | void dasd_device_clear_timer(struct dasd_device *device) | 933 | void dasd_device_clear_timer(struct dasd_device *device) |
937 | { | 934 | { |
938 | if (timer_pending(&device->timer)) | 935 | del_timer(&device->timer); |
939 | del_timer(&device->timer); | ||
940 | } | 936 | } |
941 | 937 | ||
942 | static void dasd_handle_killed_request(struct ccw_device *cdev, | 938 | static void dasd_handle_killed_request(struct ccw_device *cdev, |
@@ -1586,19 +1582,10 @@ static void dasd_block_timeout(unsigned long ptr) | |||
1586 | */ | 1582 | */ |
1587 | void dasd_block_set_timer(struct dasd_block *block, int expires) | 1583 | void dasd_block_set_timer(struct dasd_block *block, int expires) |
1588 | { | 1584 | { |
1589 | if (expires == 0) { | 1585 | if (expires == 0) |
1590 | if (timer_pending(&block->timer)) | 1586 | del_timer(&block->timer); |
1591 | del_timer(&block->timer); | 1587 | else |
1592 | return; | 1588 | mod_timer(&block->timer, jiffies + expires); |
1593 | } | ||
1594 | if (timer_pending(&block->timer)) { | ||
1595 | if (mod_timer(&block->timer, jiffies + expires)) | ||
1596 | return; | ||
1597 | } | ||
1598 | block->timer.function = dasd_block_timeout; | ||
1599 | block->timer.data = (unsigned long) block; | ||
1600 | block->timer.expires = jiffies + expires; | ||
1601 | add_timer(&block->timer); | ||
1602 | } | 1589 | } |
1603 | 1590 | ||
1604 | /* | 1591 | /* |
@@ -1606,8 +1593,7 @@ void dasd_block_set_timer(struct dasd_block *block, int expires) | |||
1606 | */ | 1593 | */ |
1607 | void dasd_block_clear_timer(struct dasd_block *block) | 1594 | void dasd_block_clear_timer(struct dasd_block *block) |
1608 | { | 1595 | { |
1609 | if (timer_pending(&block->timer)) | 1596 | del_timer(&block->timer); |
1610 | del_timer(&block->timer); | ||
1611 | } | 1597 | } |
1612 | 1598 | ||
1613 | /* | 1599 | /* |