diff options
author | Peter Oberparleiter <peter.oberparleiter@de.ibm.com> | 2008-08-21 13:46:39 -0400 |
---|---|---|
committer | Martin Schwidefsky <schwidefsky@de.ibm.com> | 2008-08-21 13:46:41 -0400 |
commit | 91c36919a456589f4f073671474a1f899e0d3c2b (patch) | |
tree | 63cb2ee1afd9b00bf2ea4959482d58f402bb21f3 /drivers/s390/cio | |
parent | 49fd38bdaa96f093fcad3176a781a4d0de8f8602 (diff) |
[S390] cio: call ccw driver notify function with lock held
Calling a ccw driver's notify function without the ccw device lock
held opens up a race window between discovery and handling of a change
in the device operational state. As a result, the device driver may
encounter unexpected device malfunction, leading to out-of-retry
situations or similar.
Remove race by extending the ccw device lock from state change
discovery to the calling of the notify function.
Signed-off-by: Peter Oberparleiter <peter.oberparleiter@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Diffstat (limited to 'drivers/s390/cio')
-rw-r--r-- | drivers/s390/cio/css.c | 1 | ||||
-rw-r--r-- | drivers/s390/cio/device.c | 40 | ||||
-rw-r--r-- | drivers/s390/cio/device.h | 2 | ||||
-rw-r--r-- | drivers/s390/cio/device_fsm.c | 31 |
4 files changed, 40 insertions, 34 deletions
diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c index 46c021d880dc..51489eff6b0b 100644 --- a/drivers/s390/cio/css.c +++ b/drivers/s390/cio/css.c | |||
@@ -477,7 +477,6 @@ void css_schedule_eval_all(void) | |||
477 | 477 | ||
478 | void css_wait_for_slow_path(void) | 478 | void css_wait_for_slow_path(void) |
479 | { | 479 | { |
480 | flush_workqueue(ccw_device_notify_work); | ||
481 | flush_workqueue(slow_path_wq); | 480 | flush_workqueue(slow_path_wq); |
482 | } | 481 | } |
483 | 482 | ||
diff --git a/drivers/s390/cio/device.c b/drivers/s390/cio/device.c index e818d0c54c09..28221030b886 100644 --- a/drivers/s390/cio/device.c +++ b/drivers/s390/cio/device.c | |||
@@ -150,7 +150,6 @@ static struct css_driver io_subchannel_driver = { | |||
150 | }; | 150 | }; |
151 | 151 | ||
152 | struct workqueue_struct *ccw_device_work; | 152 | struct workqueue_struct *ccw_device_work; |
153 | struct workqueue_struct *ccw_device_notify_work; | ||
154 | wait_queue_head_t ccw_device_init_wq; | 153 | wait_queue_head_t ccw_device_init_wq; |
155 | atomic_t ccw_device_init_count; | 154 | atomic_t ccw_device_init_count; |
156 | 155 | ||
@@ -168,11 +167,6 @@ init_ccw_bus_type (void) | |||
168 | ccw_device_work = create_singlethread_workqueue("cio"); | 167 | ccw_device_work = create_singlethread_workqueue("cio"); |
169 | if (!ccw_device_work) | 168 | if (!ccw_device_work) |
170 | return -ENOMEM; /* FIXME: better errno ? */ | 169 | return -ENOMEM; /* FIXME: better errno ? */ |
171 | ccw_device_notify_work = create_singlethread_workqueue("cio_notify"); | ||
172 | if (!ccw_device_notify_work) { | ||
173 | ret = -ENOMEM; /* FIXME: better errno ? */ | ||
174 | goto out_err; | ||
175 | } | ||
176 | slow_path_wq = create_singlethread_workqueue("kslowcrw"); | 170 | slow_path_wq = create_singlethread_workqueue("kslowcrw"); |
177 | if (!slow_path_wq) { | 171 | if (!slow_path_wq) { |
178 | ret = -ENOMEM; /* FIXME: better errno ? */ | 172 | ret = -ENOMEM; /* FIXME: better errno ? */ |
@@ -192,8 +186,6 @@ init_ccw_bus_type (void) | |||
192 | out_err: | 186 | out_err: |
193 | if (ccw_device_work) | 187 | if (ccw_device_work) |
194 | destroy_workqueue(ccw_device_work); | 188 | destroy_workqueue(ccw_device_work); |
195 | if (ccw_device_notify_work) | ||
196 | destroy_workqueue(ccw_device_notify_work); | ||
197 | if (slow_path_wq) | 189 | if (slow_path_wq) |
198 | destroy_workqueue(slow_path_wq); | 190 | destroy_workqueue(slow_path_wq); |
199 | return ret; | 191 | return ret; |
@@ -204,7 +196,6 @@ cleanup_ccw_bus_type (void) | |||
204 | { | 196 | { |
205 | css_driver_unregister(&io_subchannel_driver); | 197 | css_driver_unregister(&io_subchannel_driver); |
206 | bus_unregister(&ccw_bus_type); | 198 | bus_unregister(&ccw_bus_type); |
207 | destroy_workqueue(ccw_device_notify_work); | ||
208 | destroy_workqueue(ccw_device_work); | 199 | destroy_workqueue(ccw_device_work); |
209 | } | 200 | } |
210 | 201 | ||
@@ -1496,11 +1487,22 @@ static void device_set_disconnected(struct ccw_device *cdev) | |||
1496 | ccw_device_schedule_recovery(); | 1487 | ccw_device_schedule_recovery(); |
1497 | } | 1488 | } |
1498 | 1489 | ||
1490 | void ccw_device_set_notoper(struct ccw_device *cdev) | ||
1491 | { | ||
1492 | struct subchannel *sch = to_subchannel(cdev->dev.parent); | ||
1493 | |||
1494 | CIO_TRACE_EVENT(2, "notoper"); | ||
1495 | CIO_TRACE_EVENT(2, sch->dev.bus_id); | ||
1496 | ccw_device_set_timeout(cdev, 0); | ||
1497 | cio_disable_subchannel(sch); | ||
1498 | cdev->private->state = DEV_STATE_NOT_OPER; | ||
1499 | } | ||
1500 | |||
1499 | static int io_subchannel_sch_event(struct subchannel *sch, int slow) | 1501 | static int io_subchannel_sch_event(struct subchannel *sch, int slow) |
1500 | { | 1502 | { |
1501 | int event, ret, disc; | 1503 | int event, ret, disc; |
1502 | unsigned long flags; | 1504 | unsigned long flags; |
1503 | enum { NONE, UNREGISTER, UNREGISTER_PROBE, REPROBE } action; | 1505 | enum { NONE, UNREGISTER, UNREGISTER_PROBE, REPROBE, DISC } action; |
1504 | struct ccw_device *cdev; | 1506 | struct ccw_device *cdev; |
1505 | 1507 | ||
1506 | spin_lock_irqsave(sch->lock, flags); | 1508 | spin_lock_irqsave(sch->lock, flags); |
@@ -1535,16 +1537,11 @@ static int io_subchannel_sch_event(struct subchannel *sch, int slow) | |||
1535 | } | 1537 | } |
1536 | /* fall through */ | 1538 | /* fall through */ |
1537 | case CIO_GONE: | 1539 | case CIO_GONE: |
1538 | /* Prevent unwanted effects when opening lock. */ | ||
1539 | cio_disable_subchannel(sch); | ||
1540 | device_set_disconnected(cdev); | ||
1541 | /* Ask driver what to do with device. */ | 1540 | /* Ask driver what to do with device. */ |
1542 | action = UNREGISTER; | 1541 | if (io_subchannel_notify(sch, event)) |
1543 | spin_unlock_irqrestore(sch->lock, flags); | 1542 | action = DISC; |
1544 | ret = io_subchannel_notify(sch, event); | 1543 | else |
1545 | spin_lock_irqsave(sch->lock, flags); | 1544 | action = UNREGISTER; |
1546 | if (ret) | ||
1547 | action = NONE; | ||
1548 | break; | 1545 | break; |
1549 | case CIO_REVALIDATE: | 1546 | case CIO_REVALIDATE: |
1550 | /* Device will be removed, so no notify necessary. */ | 1547 | /* Device will be removed, so no notify necessary. */ |
@@ -1565,6 +1562,7 @@ static int io_subchannel_sch_event(struct subchannel *sch, int slow) | |||
1565 | switch (action) { | 1562 | switch (action) { |
1566 | case UNREGISTER: | 1563 | case UNREGISTER: |
1567 | case UNREGISTER_PROBE: | 1564 | case UNREGISTER_PROBE: |
1565 | ccw_device_set_notoper(cdev); | ||
1568 | /* Unregister device (will use subchannel lock). */ | 1566 | /* Unregister device (will use subchannel lock). */ |
1569 | spin_unlock_irqrestore(sch->lock, flags); | 1567 | spin_unlock_irqrestore(sch->lock, flags); |
1570 | css_sch_device_unregister(sch); | 1568 | css_sch_device_unregister(sch); |
@@ -1577,6 +1575,9 @@ static int io_subchannel_sch_event(struct subchannel *sch, int slow) | |||
1577 | case REPROBE: | 1575 | case REPROBE: |
1578 | ccw_device_trigger_reprobe(cdev); | 1576 | ccw_device_trigger_reprobe(cdev); |
1579 | break; | 1577 | break; |
1578 | case DISC: | ||
1579 | device_set_disconnected(cdev); | ||
1580 | break; | ||
1580 | default: | 1581 | default: |
1581 | break; | 1582 | break; |
1582 | } | 1583 | } |
@@ -1828,5 +1829,4 @@ EXPORT_SYMBOL(ccw_driver_unregister); | |||
1828 | EXPORT_SYMBOL(get_ccwdev_by_busid); | 1829 | EXPORT_SYMBOL(get_ccwdev_by_busid); |
1829 | EXPORT_SYMBOL(ccw_bus_type); | 1830 | EXPORT_SYMBOL(ccw_bus_type); |
1830 | EXPORT_SYMBOL(ccw_device_work); | 1831 | EXPORT_SYMBOL(ccw_device_work); |
1831 | EXPORT_SYMBOL(ccw_device_notify_work); | ||
1832 | EXPORT_SYMBOL_GPL(ccw_device_get_subchannel_id); | 1832 | EXPORT_SYMBOL_GPL(ccw_device_get_subchannel_id); |
diff --git a/drivers/s390/cio/device.h b/drivers/s390/cio/device.h index 9800a8335a3f..6f5c3f2b3587 100644 --- a/drivers/s390/cio/device.h +++ b/drivers/s390/cio/device.h | |||
@@ -72,7 +72,6 @@ dev_fsm_final_state(struct ccw_device *cdev) | |||
72 | } | 72 | } |
73 | 73 | ||
74 | extern struct workqueue_struct *ccw_device_work; | 74 | extern struct workqueue_struct *ccw_device_work; |
75 | extern struct workqueue_struct *ccw_device_notify_work; | ||
76 | extern wait_queue_head_t ccw_device_init_wq; | 75 | extern wait_queue_head_t ccw_device_init_wq; |
77 | extern atomic_t ccw_device_init_count; | 76 | extern atomic_t ccw_device_init_count; |
78 | 77 | ||
@@ -120,6 +119,7 @@ int ccw_device_stlck(struct ccw_device *); | |||
120 | void ccw_device_trigger_reprobe(struct ccw_device *); | 119 | void ccw_device_trigger_reprobe(struct ccw_device *); |
121 | void ccw_device_kill_io(struct ccw_device *); | 120 | void ccw_device_kill_io(struct ccw_device *); |
122 | int ccw_device_notify(struct ccw_device *, int); | 121 | int ccw_device_notify(struct ccw_device *, int); |
122 | void ccw_device_set_notoper(struct ccw_device *cdev); | ||
123 | 123 | ||
124 | /* qdio needs this. */ | 124 | /* qdio needs this. */ |
125 | void ccw_device_set_timeout(struct ccw_device *, int); | 125 | void ccw_device_set_timeout(struct ccw_device *, int); |
diff --git a/drivers/s390/cio/device_fsm.c b/drivers/s390/cio/device_fsm.c index 8b5fe57fb2f3..550508df952b 100644 --- a/drivers/s390/cio/device_fsm.c +++ b/drivers/s390/cio/device_fsm.c | |||
@@ -337,26 +337,34 @@ int ccw_device_notify(struct ccw_device *cdev, int event) | |||
337 | return 0; | 337 | return 0; |
338 | if (!cdev->online) | 338 | if (!cdev->online) |
339 | return 0; | 339 | return 0; |
340 | CIO_MSG_EVENT(2, "notify called for 0.%x.%04x, event=%d\n", | ||
341 | cdev->private->dev_id.ssid, cdev->private->dev_id.devno, | ||
342 | event); | ||
340 | return cdev->drv->notify ? cdev->drv->notify(cdev, event) : 0; | 343 | return cdev->drv->notify ? cdev->drv->notify(cdev, event) : 0; |
341 | } | 344 | } |
342 | 345 | ||
343 | static void | 346 | static void cmf_reenable_delayed(struct work_struct *work) |
344 | ccw_device_oper_notify(struct work_struct *work) | ||
345 | { | 347 | { |
346 | struct ccw_device_private *priv; | 348 | struct ccw_device_private *priv; |
347 | struct ccw_device *cdev; | 349 | struct ccw_device *cdev; |
348 | int ret; | ||
349 | 350 | ||
350 | priv = container_of(work, struct ccw_device_private, kick_work); | 351 | priv = container_of(work, struct ccw_device_private, kick_work); |
351 | cdev = priv->cdev; | 352 | cdev = priv->cdev; |
352 | ret = ccw_device_notify(cdev, CIO_OPER); | 353 | cmf_reenable(cdev); |
353 | if (ret) { | 354 | } |
355 | |||
356 | static void ccw_device_oper_notify(struct ccw_device *cdev) | ||
357 | { | ||
358 | if (ccw_device_notify(cdev, CIO_OPER)) { | ||
354 | /* Reenable channel measurements, if needed. */ | 359 | /* Reenable channel measurements, if needed. */ |
355 | cmf_reenable(cdev); | 360 | PREPARE_WORK(&cdev->private->kick_work, cmf_reenable_delayed); |
356 | wake_up(&cdev->private->wait_q); | 361 | queue_work(ccw_device_work, &cdev->private->kick_work); |
357 | } else | 362 | return; |
358 | /* Driver doesn't want device back. */ | 363 | } |
359 | ccw_device_do_unreg_rereg(work); | 364 | /* Driver doesn't want device back. */ |
365 | ccw_device_set_notoper(cdev); | ||
366 | PREPARE_WORK(&cdev->private->kick_work, ccw_device_do_unreg_rereg); | ||
367 | queue_work(ccw_device_work, &cdev->private->kick_work); | ||
360 | } | 368 | } |
361 | 369 | ||
362 | /* | 370 | /* |
@@ -386,8 +394,7 @@ ccw_device_done(struct ccw_device *cdev, int state) | |||
386 | 394 | ||
387 | if (cdev->private->flags.donotify) { | 395 | if (cdev->private->flags.donotify) { |
388 | cdev->private->flags.donotify = 0; | 396 | cdev->private->flags.donotify = 0; |
389 | PREPARE_WORK(&cdev->private->kick_work, ccw_device_oper_notify); | 397 | ccw_device_oper_notify(cdev); |
390 | queue_work(ccw_device_notify_work, &cdev->private->kick_work); | ||
391 | } | 398 | } |
392 | wake_up(&cdev->private->wait_q); | 399 | wake_up(&cdev->private->wait_q); |
393 | 400 | ||