[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>

diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c
index 1b6c52ef73391da1aff66cc1aa6bcda8c193ddd8..acb78017e7d0fd2e2988bfed85ec484e527bfcc6 100644 (file)
--- a/drivers/s390/block/dasd.c
+++ b/drivers/s390/block/dasd.c
@@ -2333,13 +2333,11 @@ int dasd_generic_notify(struct ccw_device *cdev, int event)
 {
        struct dasd_device *device;
        struct dasd_ccw_req *cqr;
-       unsigned long flags;
        int ret;
 
-       device = dasd_device_from_cdev(cdev);
+       device = dasd_device_from_cdev_locked(cdev);
        if (IS_ERR(device))
                return 0;
-       spin_lock_irqsave(get_ccwdev_lock(cdev), flags);
        ret = 0;
        switch (event) {
        case CIO_GONE:
@@ -2369,7 +2367,6 @@ int dasd_generic_notify(struct ccw_device *cdev, int event)
                ret = 1;
                break;
        }
-       spin_unlock_irqrestore(get_ccwdev_lock(cdev), flags);
        dasd_put_device(device);
        return ret;
 }

diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c
index 46c021d880dc5ee7441cd11f8e885bf54d79b7dd..51489eff6b0b4484ad234e6b7b7a0819a6d1ddd6 100644 (file)
--- a/drivers/s390/cio/css.c
+++ b/drivers/s390/cio/css.c
@@ -477,7 +477,6 @@ void css_schedule_eval_all(void)
 
 void css_wait_for_slow_path(void)
 {
-       flush_workqueue(ccw_device_notify_work);
        flush_workqueue(slow_path_wq);
 }
 

diff --git a/drivers/s390/cio/device.c b/drivers/s390/cio/device.c
index e818d0c54c0949f5a21ae134ccf426da5d079d9c..28221030b886cfa5ae8d794fdf176bdbfe401845 100644 (file)
--- a/drivers/s390/cio/device.c
+++ b/drivers/s390/cio/device.c
@@ -150,7 +150,6 @@ static struct css_driver io_subchannel_driver = {
 };
 
 struct workqueue_struct *ccw_device_work;
-struct workqueue_struct *ccw_device_notify_work;
 wait_queue_head_t ccw_device_init_wq;
 atomic_t ccw_device_init_count;
 
@@ -168,11 +167,6 @@ init_ccw_bus_type (void)
        ccw_device_work = create_singlethread_workqueue("cio");
        if (!ccw_device_work)
                return -ENOMEM; /* FIXME: better errno ? */
-       ccw_device_notify_work = create_singlethread_workqueue("cio_notify");
-       if (!ccw_device_notify_work) {
-               ret = -ENOMEM; /* FIXME: better errno ? */
-               goto out_err;
-       }
        slow_path_wq = create_singlethread_workqueue("kslowcrw");
        if (!slow_path_wq) {
                ret = -ENOMEM; /* FIXME: better errno ? */
@@ -192,8 +186,6 @@ init_ccw_bus_type (void)
 out_err:
        if (ccw_device_work)
                destroy_workqueue(ccw_device_work);
-       if (ccw_device_notify_work)
-               destroy_workqueue(ccw_device_notify_work);
        if (slow_path_wq)
                destroy_workqueue(slow_path_wq);
        return ret;
@@ -204,7 +196,6 @@ cleanup_ccw_bus_type (void)
 {
        css_driver_unregister(&io_subchannel_driver);
        bus_unregister(&ccw_bus_type);
-       destroy_workqueue(ccw_device_notify_work);
        destroy_workqueue(ccw_device_work);
 }
 
@@ -1496,11 +1487,22 @@ static void device_set_disconnected(struct ccw_device *cdev)
                ccw_device_schedule_recovery();
 }
 
+void ccw_device_set_notoper(struct ccw_device *cdev)
+{
+       struct subchannel *sch = to_subchannel(cdev->dev.parent);
+
+       CIO_TRACE_EVENT(2, "notoper");
+       CIO_TRACE_EVENT(2, sch->dev.bus_id);
+       ccw_device_set_timeout(cdev, 0);
+       cio_disable_subchannel(sch);
+       cdev->private->state = DEV_STATE_NOT_OPER;
+}
+
 static int io_subchannel_sch_event(struct subchannel *sch, int slow)
 {
        int event, ret, disc;
        unsigned long flags;
-       enum { NONE, UNREGISTER, UNREGISTER_PROBE, REPROBE } action;
+       enum { NONE, UNREGISTER, UNREGISTER_PROBE, REPROBE, DISC } action;
        struct ccw_device *cdev;
 
        spin_lock_irqsave(sch->lock, flags);
@@ -1535,16 +1537,11 @@ static int io_subchannel_sch_event(struct subchannel *sch, int slow)
                }
                /* fall through */
        case CIO_GONE:
-               /* Prevent unwanted effects when opening lock. */
-               cio_disable_subchannel(sch);
-               device_set_disconnected(cdev);
                /* Ask driver what to do with device. */
-               action = UNREGISTER;
-               spin_unlock_irqrestore(sch->lock, flags);
-               ret = io_subchannel_notify(sch, event);
-               spin_lock_irqsave(sch->lock, flags);
-               if (ret)
-                       action = NONE;
+               if (io_subchannel_notify(sch, event))
+                       action = DISC;
+               else
+                       action = UNREGISTER;
                break;
        case CIO_REVALIDATE:
                /* Device will be removed, so no notify necessary. */
@@ -1565,6 +1562,7 @@ static int io_subchannel_sch_event(struct subchannel *sch, int slow)
        switch (action) {
        case UNREGISTER:
        case UNREGISTER_PROBE:
+               ccw_device_set_notoper(cdev);
                /* Unregister device (will use subchannel lock). */
                spin_unlock_irqrestore(sch->lock, flags);
                css_sch_device_unregister(sch);
@@ -1577,6 +1575,9 @@ static int io_subchannel_sch_event(struct subchannel *sch, int slow)
        case REPROBE:
                ccw_device_trigger_reprobe(cdev);
                break;
+       case DISC:
+               device_set_disconnected(cdev);
+               break;
        default:
                break;
        }
@@ -1828,5 +1829,4 @@ EXPORT_SYMBOL(ccw_driver_unregister);
 EXPORT_SYMBOL(get_ccwdev_by_busid);
 EXPORT_SYMBOL(ccw_bus_type);
 EXPORT_SYMBOL(ccw_device_work);
-EXPORT_SYMBOL(ccw_device_notify_work);
 EXPORT_SYMBOL_GPL(ccw_device_get_subchannel_id);

diff --git a/drivers/s390/cio/device.h b/drivers/s390/cio/device.h
index 9800a8335a3fa81a195b9c5b3ae83380c21ae5ef..6f5c3f2b3587d8f72d98aa781f9f2cbd6dfd50d3 100644 (file)
--- a/drivers/s390/cio/device.h
+++ b/drivers/s390/cio/device.h
@@ -72,7 +72,6 @@ dev_fsm_final_state(struct ccw_device *cdev)
 }
 
 extern struct workqueue_struct *ccw_device_work;
-extern struct workqueue_struct *ccw_device_notify_work;
 extern wait_queue_head_t ccw_device_init_wq;
 extern atomic_t ccw_device_init_count;
 
@@ -120,6 +119,7 @@ int ccw_device_stlck(struct ccw_device *);
 void ccw_device_trigger_reprobe(struct ccw_device *);
 void ccw_device_kill_io(struct ccw_device *);
 int ccw_device_notify(struct ccw_device *, int);
+void ccw_device_set_notoper(struct ccw_device *cdev);
 
 /* qdio needs this. */
 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 8b5fe57fb2f31ef1684d586aca9f69c9c9e4fab3..550508df952b89693f5b13e4de5e5bec2a976ac3 100644 (file)
--- 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)
                return 0;
        if (!cdev->online)
                return 0;
+       CIO_MSG_EVENT(2, "notify called for 0.%x.%04x, event=%d\n",
+                     cdev->private->dev_id.ssid, cdev->private->dev_id.devno,
+                     event);
        return cdev->drv->notify ? cdev->drv->notify(cdev, event) : 0;
 }
 
-static void
-ccw_device_oper_notify(struct work_struct *work)
+static void cmf_reenable_delayed(struct work_struct *work)
 {
        struct ccw_device_private *priv;
        struct ccw_device *cdev;
-       int ret;
 
        priv = container_of(work, struct ccw_device_private, kick_work);
        cdev = priv->cdev;
-       ret = ccw_device_notify(cdev, CIO_OPER);
-       if (ret) {
+       cmf_reenable(cdev);
+}
+
+static void ccw_device_oper_notify(struct ccw_device *cdev)
+{
+       if (ccw_device_notify(cdev, CIO_OPER)) {
                /* Reenable channel measurements, if needed. */
-               cmf_reenable(cdev);
-               wake_up(&cdev->private->wait_q);
-       } else
-               /* Driver doesn't want device back. */
-               ccw_device_do_unreg_rereg(work);
+               PREPARE_WORK(&cdev->private->kick_work, cmf_reenable_delayed);
+               queue_work(ccw_device_work, &cdev->private->kick_work);
+               return;
+       }
+       /* Driver doesn't want device back. */
+       ccw_device_set_notoper(cdev);
+       PREPARE_WORK(&cdev->private->kick_work, ccw_device_do_unreg_rereg);
+       queue_work(ccw_device_work, &cdev->private->kick_work);
 }
 
 /*
@@ -386,8 +394,7 @@ ccw_device_done(struct ccw_device *cdev, int state)
 
        if (cdev->private->flags.donotify) {
                cdev->private->flags.donotify = 0;
-               PREPARE_WORK(&cdev->private->kick_work, ccw_device_oper_notify);
-               queue_work(ccw_device_notify_work, &cdev->private->kick_work);
+               ccw_device_oper_notify(cdev);
        }
        wake_up(&cdev->private->wait_q);