isci: replace isci_remote_device completion with event queue

Replace the device completion infrastructure with the controller wide
event queue.  There was a potential for the stop and ready notifications
to corrupt each other, now that cannot happen.

The stop pending flag cannot be used until devices are statically
allocated.  We temporarily need to maintain a completion to handle
waiting for an object that has disappeared, but we can at least stop
scribbling on freed memory.

A future change will also get rid of the "stopping" state as it should
not be exposed to the rest of the driver.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>

diff --git a/drivers/scsi/isci/events.c b/drivers/scsi/isci/events.c
index c5cbaedac041ae56724062ac6a30045bb89317b2..9d58e458a37bb556f4257bd83ecbf3df10b75dc1 100644 (file)
--- a/drivers/scsi/isci/events.c
+++ b/drivers/scsi/isci/events.c
@@ -526,7 +526,7 @@ void isci_event_remote_device_start_complete(
 /**
  * isci_event_remote_device_stop_complete() - This user callback method will
  *    inform the user that a stop operation has completed.
- * @controller: This parameter specifies the core controller associated with
+ * @scic: This parameter specifies the core controller associated with
  *    the completion callback.
  * @remote_device: This parameter specifies the remote device associated with
  *    the completion callback.
@@ -534,28 +534,20 @@ void isci_event_remote_device_start_complete(
  *    operation.
  *
  */
-void isci_event_remote_device_stop_complete(
-       struct scic_sds_controller *controller,
-       struct scic_sds_remote_device *remote_device,
-       enum sci_status completion_status)
+void isci_event_remote_device_stop_complete(struct scic_sds_controller *scic,
+                                           struct scic_sds_remote_device *sci_dev,
+                                           enum sci_status completion_status)
 {
-       struct isci_host *isci_host;
-       struct isci_remote_device *isci_device;
+       struct isci_host *ihost;
+       struct isci_remote_device *idev;
 
-       isci_host =
-               (struct isci_host *)sci_object_get_association(controller);
+       ihost = sci_object_get_association(scic);
+       idev = sci_object_get_association(sci_dev);
 
-       isci_device =
-               (struct isci_remote_device *)sci_object_get_association(
-                       remote_device
-                       );
-
-       dev_dbg(&isci_host->pdev->dev,
-               "%s: isci_device = %p\n", __func__, isci_device);
-
-       isci_remote_device_stop_complete(
-               isci_host, isci_device, completion_status);
+       dev_dbg(&ihost->pdev->dev,
+               "%s: idev = %p\n", __func__, idev);
 
+       isci_remote_device_stop_complete(ihost, idev, completion_status);
 }
 
 /**

diff --git a/drivers/scsi/isci/host.c b/drivers/scsi/isci/host.c
index da0c0da4198fb9906caa78e888d60f5094613d37..8d255666a657d211d7af17e4e24d56bf4d79744a 100644 (file)
--- a/drivers/scsi/isci/host.c
+++ b/drivers/scsi/isci/host.c
@@ -345,7 +345,7 @@ void isci_host_deinit(struct isci_host *ihost)
 
                list_for_each_entry_safe(idev, d, &port->remote_dev_list, node) {
                        isci_remote_device_change_state(idev, isci_stopping);
-                       isci_remote_device_stop(idev);
+                       isci_remote_device_stop(ihost, idev);
                }
        }
 

diff --git a/drivers/scsi/isci/host.h b/drivers/scsi/isci/host.h
index 7c1f0b5cee7ddc430852fa26a4cd4e1388bfb7dc..6a6304c06976b0c4c0ff23edaa2dd80784b9c683 100644 (file)
--- a/drivers/scsi/isci/host.h
+++ b/drivers/scsi/isci/host.h
@@ -212,6 +212,19 @@ static inline void wait_for_stop(struct isci_host *ihost)
        wait_event(ihost->eventq, !test_bit(IHOST_STOP_PENDING, &ihost->flags));
 }
 
+static inline void wait_for_device_start(struct isci_host *ihost, struct isci_remote_device *idev)
+{
+       wait_event(ihost->eventq, !test_bit(IDEV_START_PENDING, &idev->flags));
+}
+
+static inline void wait_for_device_stop(struct isci_host *ihost, struct isci_remote_device *idev)
+{
+       /* todo switch to:
+        * wait_event(ihost->eventq, !test_bit(IDEV_STOP_PENDING, &idev->flags));
+        * once devices are statically allocated
+        */
+       wait_for_completion(idev->cmp);
+}
 
 /**
  * isci_host_from_sas_ha() - This accessor retrieves the isci_host object

diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c
index fc1f2444917042cd192506c00c2741f1cde5cedb..db2259ce003f58a849f628ebed4411e0a272a184 100644 (file)
--- a/drivers/scsi/isci/remote_device.c
+++ b/drivers/scsi/isci/remote_device.c
@@ -95,6 +95,11 @@ static void isci_remote_device_deconstruct(
        scic_remote_device_destruct(to_sci_dev(isci_device));
        isci_device->domain_dev->lldd_dev = NULL;
        list_del(&isci_device->node);
+
+       clear_bit(IDEV_STOP_PENDING, &isci_device->flags);
+       clear_bit(IDEV_START_PENDING, &isci_device->flags);
+       wake_up(&isci_host->eventq);
+       complete(isci_device->cmp);
        kmem_cache_free(isci_kmem_cache, isci_device);
 }
 
@@ -279,30 +284,22 @@ isci_remote_device_alloc(struct isci_host *isci_host, struct isci_port *port)
  * isci_remote_device_ready() - This function is called by the scic when the
  *    remote device is ready. We mark the isci device as ready and signal the
  *    waiting proccess.
- * @isci_host: This parameter specifies the isci host object.
- * @isci_device: This parameter specifies the remote device
+ * @idev: This parameter specifies the remote device
  *
  */
-void isci_remote_device_ready(struct isci_remote_device *isci_device)
+void isci_remote_device_ready(struct isci_remote_device *idev)
 {
+       struct isci_host *ihost = idev->isci_port->isci_host;
        unsigned long flags;
 
-       dev_dbg(&isci_device->isci_port->isci_host->pdev->dev,
-               "%s: isci_device = %p\n", __func__, isci_device);
-
-       /* device ready is actually a "ready for io" state. */
-       if (isci_device->status == isci_starting ||
-           isci_device->status == isci_ready) {
-               spin_lock_irqsave(&isci_device->isci_port->remote_device_lock,
-                                 flags);
-               isci_remote_device_change_state(isci_device, isci_ready_for_io);
-               if (isci_device->completion)
-                       complete(isci_device->completion);
-               spin_unlock_irqrestore(
-                               &isci_device->isci_port->remote_device_lock,
-                               flags);
-       }
+       dev_dbg(&ihost->pdev->dev,
+               "%s: isci_device = %p\n", __func__, idev);
 
+       spin_lock_irqsave(&idev->isci_port->remote_device_lock, flags);
+       isci_remote_device_change_state(idev, isci_ready_for_io);
+       if (test_and_clear_bit(IDEV_START_PENDING, &idev->flags))
+               wake_up(&ihost->eventq);
+       spin_unlock_irqrestore(&idev->isci_port->remote_device_lock, flags);
 }
 
 /**
@@ -341,8 +338,6 @@ void isci_remote_device_stop_complete(
        struct isci_remote_device *isci_device,
        enum sci_status status)
 {
-       struct completion *completion = isci_device->completion;
-
        dev_dbg(&isci_host->pdev->dev,
                "%s: complete isci_device = %p, status = 0x%x\n",
                __func__,
@@ -354,9 +349,6 @@ void isci_remote_device_stop_complete(
        /* after stop, we can tear down resources. */
        isci_remote_device_deconstruct(isci_host, isci_device);
 
-       /* notify interested parties. */
-       if (completion)
-               complete(completion);
 }
 
 /**
@@ -385,40 +377,33 @@ void isci_remote_device_start_complete(
  *
  * The status of the scic request to stop.
  */
-enum sci_status isci_remote_device_stop(
-       struct isci_remote_device *isci_device)
+enum sci_status isci_remote_device_stop(struct isci_host *ihost, struct isci_remote_device *idev)
 {
        enum sci_status status;
        unsigned long flags;
-
        DECLARE_COMPLETION_ONSTACK(completion);
 
-       dev_dbg(&isci_device->isci_port->isci_host->pdev->dev,
-               "%s: isci_device = %p\n", __func__, isci_device);
-
-       isci_remote_device_change_state(isci_device, isci_stopping);
-
-       /* We need comfirmation that stop completed. */
-       isci_device->completion = &completion;
+       dev_dbg(&ihost->pdev->dev,
+               "%s: isci_device = %p\n", __func__, idev);
 
-       BUG_ON(isci_device->isci_port == NULL);
-       BUG_ON(isci_device->isci_port->isci_host == NULL);
+       isci_remote_device_change_state(idev, isci_stopping);
+       set_bit(IDEV_STOP_PENDING, &idev->flags);
+       idev->cmp = &completion;
 
-       spin_lock_irqsave(&isci_device->isci_port->isci_host->scic_lock, flags);
+       spin_lock_irqsave(&ihost->scic_lock, flags);
 
-       status = scic_remote_device_stop(to_sci_dev(isci_device), 50);
+       status = scic_remote_device_stop(to_sci_dev(idev), 50);
 
-       spin_unlock_irqrestore(&isci_device->isci_port->isci_host->scic_lock, flags);
+       spin_unlock_irqrestore(&ihost->scic_lock, flags);
 
        /* Wait for the stop complete callback. */
        if (status == SCI_SUCCESS)
-               wait_for_completion(&completion);
+               wait_for_device_stop(ihost, idev);
 
-       dev_dbg(&isci_device->isci_port->isci_host->pdev->dev,
-               "%s: isci_device = %p - after completion wait\n",
-               __func__, isci_device);
+       dev_dbg(&ihost->pdev->dev,
+               "%s: idev = %p - after completion wait\n",
+               __func__, idev);
 
-       isci_device->completion = NULL;
        return status;
 }
 
@@ -428,18 +413,16 @@ enum sci_status isci_remote_device_stop(
  * @domain_device: This parameter specifies the libsas domain device.
  *
  */
-void isci_remote_device_gone(
-       struct domain_device *domain_dev)
+void isci_remote_device_gone(struct domain_device *dev)
 {
-       struct isci_remote_device *isci_device = isci_dev_from_domain_dev(
-               domain_dev);
+       struct isci_host *ihost = dev->port->ha->lldd_ha;
+       struct isci_remote_device *idev = dev->lldd_dev;
 
-       dev_dbg(&isci_device->isci_port->isci_host->pdev->dev,
+       dev_dbg(&ihost->pdev->dev,
                "%s: domain_device = %p, isci_device = %p, isci_port = %p\n",
-               __func__, domain_dev, isci_device, isci_device->isci_port);
+               __func__, dev, idev, idev->isci_port);
 
-       if (isci_device != NULL)
-               isci_remote_device_stop(isci_device);
+       isci_remote_device_stop(ihost, idev);
 }
 
 
@@ -462,7 +445,6 @@ int isci_remote_device_found(struct domain_device *domain_dev)
        struct asd_sas_phy *sas_phy;
        struct isci_remote_device *isci_device;
        enum sci_status status;
-       DECLARE_COMPLETION_ONSTACK(completion);
 
        isci_host = isci_host_from_sas_ha(domain_dev->port->ha);
 
@@ -498,17 +480,10 @@ int isci_remote_device_found(struct domain_device *domain_dev)
        spin_lock_irqsave(&isci_port->remote_device_lock, flags);
        list_add_tail(&isci_device->node, &isci_port->remote_dev_list);
 
-       /* for the device ready event. */
-       isci_device->completion = &completion;
-
+       set_bit(IDEV_START_PENDING, &isci_device->flags);
        status = isci_remote_device_construct(isci_port, isci_device);
-
        spin_unlock_irqrestore(&isci_port->remote_device_lock, flags);
 
-       /* wait for the device ready callback. */
-       wait_for_completion(isci_device->completion);
-       isci_device->completion = NULL;
-
        dev_dbg(&isci_host->pdev->dev,
                "%s: isci_device = %p\n",
                __func__, isci_device);
@@ -524,6 +499,9 @@ int isci_remote_device_found(struct domain_device *domain_dev)
                return -ENODEV;
        }
 
+       /* wait for the device ready callback. */
+       wait_for_device_start(isci_host, isci_device);
+
        return 0;
 }
 /**

diff --git a/drivers/scsi/isci/remote_device.h b/drivers/scsi/isci/remote_device.h
index af03039c12f1858c742606d33f5dbc04385402b3..3c22137c9f65646cbce9073c199432e69c2d6546 100644 (file)
--- a/drivers/scsi/isci/remote_device.h
+++ b/drivers/scsi/isci/remote_device.h
@@ -61,12 +61,14 @@ struct scic_sds_remote_device;
 
 struct isci_remote_device {
        enum isci_status status;
+       #define IDEV_START_PENDING 0
+       #define IDEV_STOP_PENDING 1
+       unsigned long flags;
+       struct completion *cmp;
        struct isci_port *isci_port;
        struct domain_device *domain_dev;
-       struct completion *completion;
        struct list_head node;
        struct list_head reqs_in_process;
-       struct work_struct stop_work;
        spinlock_t state_lock;
 };
 
@@ -102,9 +104,8 @@ void isci_remote_device_stop_complete(
        struct isci_remote_device *,
        enum sci_status);
 
-enum sci_status isci_remote_device_stop(
-       struct isci_remote_device *isci_device);
-
+enum sci_status isci_remote_device_stop(struct isci_host *ihost,
+                                       struct isci_remote_device *idev);
 void isci_remote_device_nuke_requests(
        struct isci_remote_device *isci_device);