[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>
This commit is contained in:
Peter Oberparleiter 2008-08-21 19:46:39 +02:00 committed by Martin Schwidefsky
parent 49fd38bdaa
commit 91c36919a4
5 changed files with 41 additions and 38 deletions

View File

@ -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;
}

View File

@ -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);
}

View File

@ -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);

View File

@ -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);

View File

@ -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);