isci: preallocate remote devices

Until we synchronize against device removal this limits the damage of
use after free bugs to the driver's own objects.  Unless we implement
reference counting we need to ensure at least a subset of a remote
device is valid at all times.  We follow the lead of other libsas
drivers that also preallocate devices.

This also enforces maximum remote device accounting at the lldd layer,
but the core may still run out of RNC's before we hit this limit.

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

diff --git a/drivers/scsi/isci/host.c b/drivers/scsi/isci/host.c
index 8d255666a657d211d7af17e4e24d56bf4d79744a..ae5d4602207354cabb124e85409197f3bdf60871 100644 (file)
--- a/drivers/scsi/isci/host.c
+++ b/drivers/scsi/isci/host.c
@@ -378,8 +378,7 @@ static void __iomem *smu_base(struct isci_host *isci_host)
 
 int isci_host_init(struct isci_host *isci_host)
 {
-       int err = 0;
-       int index = 0;
+       int err = 0, i;
        enum sci_status status;
        struct scic_sds_controller *controller;
        union scic_oem_parameters scic_oem_params;
@@ -509,13 +508,19 @@ int isci_host_init(struct isci_host *isci_host)
        if (!isci_host->dma_pool)
                return -ENOMEM;
 
-       for (index = 0; index < SCI_MAX_PORTS; index++)
-               isci_port_init(&isci_host->isci_ports[index],
-                              isci_host,
-                              index);
+       for (i = 0; i < SCI_MAX_PORTS; i++)
+               isci_port_init(&isci_host->isci_ports[i], isci_host, i);
 
-       for (index = 0; index < SCI_MAX_PHYS; index++)
-               isci_phy_init(&isci_host->phys[index], isci_host, index);
+       for (i = 0; i < SCI_MAX_PHYS; i++)
+               isci_phy_init(&isci_host->phys[i], isci_host, i);
+
+       for (i = 0; i < SCI_MAX_REMOTE_DEVICES; i++) {
+               struct isci_remote_device *idev = idev_by_id(isci_host, i);
+
+               INIT_LIST_HEAD(&idev->reqs_in_process);
+               INIT_LIST_HEAD(&idev->node);
+               spin_lock_init(&idev->state_lock);
+       }
 
        return 0;
 }

diff --git a/drivers/scsi/isci/host.h b/drivers/scsi/isci/host.h
index 6a6304c06976b0c4c0ff23edaa2dd80784b9c683..3c69f1ffb1c357628ec0c3253585fd90f440bc44 100644 (file)
--- a/drivers/scsi/isci/host.h
+++ b/drivers/scsi/isci/host.h
@@ -61,6 +61,7 @@
 /*#include "task.h"*/
 #include "timers.h"
 #include "remote_device.h"
+#include "scic_remote_device.h"
 
 #define DRV_NAME "isci"
 #define SCI_PCI_BAR_COUNT 2
@@ -117,8 +118,18 @@ struct isci_host {
        struct list_head requests_to_complete;
        struct list_head requests_to_abort;
        spinlock_t scic_lock;
+
+       /* careful only access this via idev_by_id */
+       struct isci_remote_device devices[0];
 };
 
+static inline struct isci_remote_device *idev_by_id(struct isci_host *ihost, int i)
+{
+       void *p = ihost->devices;
+
+       return p + i * (sizeof(struct isci_remote_device) +
+                       scic_remote_device_get_object_size());
+}
 
 /**
  * struct isci_pci_info - This class represents the pci function containing the
@@ -219,11 +230,7 @@ static inline void wait_for_device_start(struct isci_host *ihost, struct isci_re
 
 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);
+       wait_event(ihost->eventq, !test_bit(IDEV_STOP_PENDING, &idev->flags));
 }
 
 /**

diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c
index f1b8a51dd49f8b1cb4ea49f45fa726d4cdd12c90..2838beff43b81ab00fe78696f237eea226d6d211 100644 (file)
--- a/drivers/scsi/isci/init.c
+++ b/drivers/scsi/isci/init.c
@@ -64,7 +64,6 @@
 #include "sci_environment.h"
 
 static struct scsi_transport_template *isci_transport_template;
-struct kmem_cache *isci_kmem_cache;
 
 static DEFINE_PCI_DEVICE_TABLE(isci_id_table) = {
        { PCI_VDEVICE(INTEL, 0x1D61),},
@@ -443,7 +442,10 @@ static struct isci_host *isci_host_alloc(struct pci_dev *pdev, int id)
        struct Scsi_Host *shost;
        int err;
 
-       isci_host = devm_kzalloc(&pdev->dev, sizeof(*isci_host), GFP_KERNEL);
+       isci_host = devm_kzalloc(&pdev->dev, sizeof(*isci_host) +
+                                SCI_MAX_REMOTE_DEVICES *
+                                (sizeof(struct isci_remote_device) +
+                                 scic_remote_device_get_object_size()), GFP_KERNEL);
        if (!isci_host)
                return NULL;
 
@@ -656,31 +658,17 @@ static void __devexit isci_pci_remove(struct pci_dev *pdev)
 
 static __init int isci_init(void)
 {
-       int err = -ENOMEM;
+       int err;
 
        pr_info("%s: Intel(R) C600 SAS Controller Driver\n", DRV_NAME);
 
-       isci_kmem_cache = kmem_cache_create(DRV_NAME,
-                                           sizeof(struct isci_remote_device) +
-                                           scic_remote_device_get_object_size(),
-                                           0, 0, NULL);
-       if (!isci_kmem_cache)
-               return err;
-
        isci_transport_template = sas_domain_attach_transport(&isci_transport_ops);
        if (!isci_transport_template)
-               goto err_kmem;
+               return -ENOMEM;
 
        err = pci_register_driver(&isci_pci_driver);
        if (err)
-               goto err_sas;
-
-       return 0;
-
- err_sas:
-       sas_release_transport(isci_transport_template);
- err_kmem:
-       kmem_cache_destroy(isci_kmem_cache);
+               sas_release_transport(isci_transport_template);
 
        return err;
 }
@@ -689,7 +677,6 @@ static __exit void isci_exit(void)
 {
        pci_unregister_driver(&isci_pci_driver);
        sas_release_transport(isci_transport_template);
-       kmem_cache_destroy(isci_kmem_cache);
 }
 
 MODULE_LICENSE("Dual BSD/GPL");

diff --git a/drivers/scsi/isci/isci.h b/drivers/scsi/isci/isci.h
index 9b9aa50954cede17a2116973509de2e59d292514..24c67b039d4af2ae996eb649a1ac4d9734da47ef 100644 (file)
--- a/drivers/scsi/isci/isci.h
+++ b/drivers/scsi/isci/isci.h
@@ -89,7 +89,6 @@
 #include "task.h"
 #include "sata.h"
 
-extern struct kmem_cache *isci_kmem_cache;
 extern struct isci_firmware *isci_firmware;
 
 #define ISCI_FW_NAME           "isci/isci_firmware.bin"

diff --git a/drivers/scsi/isci/remote_device.c b/drivers/scsi/isci/remote_device.c
index db2259ce003f58a849f628ebed4411e0a272a184..48556e47bb9df01a5b0d79254f9d9c2e92668ce9 100644 (file)
--- a/drivers/scsi/isci/remote_device.c
+++ b/drivers/scsi/isci/remote_device.c
@@ -67,40 +67,35 @@
 
 /**
  * isci_remote_device_deconstruct() - This function frees an isci_remote_device.
- * @isci_host: This parameter specifies the isci host object.
- * @isci_device: This parameter specifies the remote device to be freed.
+ * @ihost: This parameter specifies the isci host object.
+ * @idev: This parameter specifies the remote device to be freed.
  *
  */
-static void isci_remote_device_deconstruct(
-       struct isci_host *isci_host,
-       struct isci_remote_device *isci_device)
+static void isci_remote_device_deconstruct(struct isci_host *ihost, struct isci_remote_device *idev)
 {
-       dev_dbg(&isci_host->pdev->dev,
-               "%s: isci_device = %p\n", __func__, isci_device);
+       dev_dbg(&ihost->pdev->dev,
+               "%s: isci_device = %p\n", __func__, idev);
 
        /* There should not be any outstanding io's. All paths to
         * here should go through isci_remote_device_nuke_requests.
         * If we hit this condition, we will need a way to complete
         * io requests in process */
-       while (!list_empty(&isci_device->reqs_in_process)) {
+       while (!list_empty(&idev->reqs_in_process)) {
 
-               dev_err(&isci_host->pdev->dev,
+               dev_err(&ihost->pdev->dev,
                        "%s: ** request list not empty! **\n", __func__);
                BUG();
        }
 
-       /* Remove all related references to this device and free
-        * the cache object.
-        */
-       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);
+       scic_remote_device_destruct(to_sci_dev(idev));
+       idev->domain_dev->lldd_dev = NULL;
+       idev->domain_dev = NULL;
+       idev->isci_port = NULL;
+       list_del_init(&idev->node);
+
+       clear_bit(IDEV_START_PENDING, &idev->flags);
+       clear_bit(IDEV_STOP_PENDING, &idev->flags);
+       wake_up(&ihost->eventq);
 }
 
 
@@ -259,25 +254,27 @@ void isci_remote_device_nuke_requests(
  * pointer to new isci_remote_device.
  */
 static struct isci_remote_device *
-isci_remote_device_alloc(struct isci_host *isci_host, struct isci_port *port)
+isci_remote_device_alloc(struct isci_host *ihost, struct isci_port *iport)
 {
-       struct isci_remote_device *isci_device;
+       struct isci_remote_device *idev;
+       int i;
 
-       isci_device = kmem_cache_zalloc(isci_kmem_cache, GFP_KERNEL);
+       for (i = 0; i < SCI_MAX_REMOTE_DEVICES; i++) {
+               idev = idev_by_id(ihost, i);
+               if (!test_and_set_bit(IDEV_ALLOCATED, &idev->flags))
+                       break;
+       }
 
-       if (!isci_device) {
-               dev_warn(&isci_host->pdev->dev, "%s: failed\n", __func__);
+       if (i >= SCI_MAX_REMOTE_DEVICES) {
+               dev_warn(&ihost->pdev->dev, "%s: failed\n", __func__);
                return NULL;
        }
 
-       INIT_LIST_HEAD(&isci_device->reqs_in_process);
-       INIT_LIST_HEAD(&isci_device->node);
-
-       spin_lock_init(&isci_device->state_lock);
-       isci_remote_device_change_state(isci_device, isci_freed);
-
-       return isci_device;
+       BUG_ON(!list_empty(&idev->reqs_in_process));
+       BUG_ON(!list_empty(&idev->node));
+       isci_remote_device_change_state(idev, isci_freed);
 
+       return idev;
 }
 
 /**
@@ -381,24 +378,22 @@ enum sci_status isci_remote_device_stop(struct isci_host *ihost, struct isci_rem
 {
        enum sci_status status;
        unsigned long flags;
-       DECLARE_COMPLETION_ONSTACK(completion);
 
        dev_dbg(&ihost->pdev->dev,
                "%s: isci_device = %p\n", __func__, idev);
 
        isci_remote_device_change_state(idev, isci_stopping);
        set_bit(IDEV_STOP_PENDING, &idev->flags);
-       idev->cmp = &completion;
 
        spin_lock_irqsave(&ihost->scic_lock, flags);
-
        status = scic_remote_device_stop(to_sci_dev(idev), 50);
-
        spin_unlock_irqrestore(&ihost->scic_lock, flags);
 
        /* Wait for the stop complete callback. */
-       if (status == SCI_SUCCESS)
+       if (status == SCI_SUCCESS) {
                wait_for_device_stop(ihost, idev);
+               clear_bit(IDEV_ALLOCATED, &idev->flags);
+       }
 
        dev_dbg(&ihost->pdev->dev,
                "%s: idev = %p - after completion wait\n",
@@ -469,6 +464,8 @@ int isci_remote_device_found(struct domain_device *domain_dev)
                return -ENODEV;
 
        isci_device = isci_remote_device_alloc(isci_host, isci_port);
+       if (!isci_device)
+               return -ENODEV;
 
        INIT_LIST_HEAD(&isci_device->node);
        domain_dev->lldd_dev = isci_device;

diff --git a/drivers/scsi/isci/remote_device.h b/drivers/scsi/isci/remote_device.h
index 3c22137c9f65646cbce9073c199432e69c2d6546..f45a5f064fce33066353df8d4db9671719d89ca7 100644 (file)
--- a/drivers/scsi/isci/remote_device.h
+++ b/drivers/scsi/isci/remote_device.h
@@ -63,8 +63,8 @@ struct isci_remote_device {
        enum isci_status status;
        #define IDEV_START_PENDING 0
        #define IDEV_STOP_PENDING 1
+       #define IDEV_ALLOCATED 2
        unsigned long flags;
-       struct completion *cmp;
        struct isci_port *isci_port;
        struct domain_device *domain_dev;
        struct list_head node;