isci: rework timer api
authorDan Williams <dan.j.williams@intel.com>
Wed, 2 Mar 2011 19:49:26 +0000 (11:49 -0800)
committerDan Williams <dan.j.williams@intel.com>
Sun, 3 Jul 2011 10:55:28 +0000 (03:55 -0700)
Prepare the timer api for the arrival of dynamic creation and
destruction events from the core.  It pretended to do this previously
but the core to date only used it in a static init-time only fashion.
This is an interim fix until a cleaner event queue can be developed.

1/ make all locking external to the api (add WARN_ONCE to verify)
2/ add a timer_destroy interface (to be used by the core)
3/ use del_timer_sync() prior to deallocating timer data
4/ delete the "timer_list" indirection, we only have timers allocated
   for the isci_host
5/ fix detection of timer list allocation errors

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
drivers/scsi/isci/events.c
drivers/scsi/isci/events.h
drivers/scsi/isci/host.c
drivers/scsi/isci/host.h
drivers/scsi/isci/task.c
drivers/scsi/isci/timers.c
drivers/scsi/isci/timers.h

index 8872f4ca6aaaf3c7f82594feee5e144bedfa3411..c5cbaedac041ae56724062ac6a30045bb89317b2 100644 (file)
  *    whenever the timer expires.
  * @controller: This parameter specifies the controller with which this timer
  *    is to be associated.
- * @cookie: This parameter specifies a piece of information that the user must
- *    retain.  This cookie is to be supplied by the user anytime a timeout
- *    occurs for the created timer.
+ * @cb_param: opaque callback parameter
  *
  * This method returns a handle to a timer object created by the user.  The
  * handle will be utilized for all further interactions relating to this timer.
  */
-void *isci_event_timer_create(
-       struct scic_sds_controller *controller,
-       void (*timer_callback)(void *),
-       void *cookie)
+void *isci_event_timer_create(struct scic_sds_controller *scic,
+                             void (*timer_callback)(void *),
+                             void *cb_param)
 {
-       struct isci_host *isci_host;
-       struct isci_timer *timer = NULL;
-
-       isci_host = (struct isci_host *)sci_object_get_association(controller);
+       struct isci_host *ihost = sci_object_get_association(scic);
+       struct isci_timer *itimer;
 
-       dev_dbg(&isci_host->pdev->dev,
-               "%s: isci_host = %p",
-               __func__, isci_host);
-
-       timer = isci_timer_create(&isci_host->timer_list_struct,
-                                 isci_host,
-                                 cookie,
-                                 timer_callback);
+       itimer = isci_timer_create(ihost, cb_param, timer_callback);
 
-       dev_dbg(&isci_host->pdev->dev, "%s: timer = %p\n", __func__, timer);
+       dev_dbg(&ihost->pdev->dev, "%s: timer = %p\n", __func__, itimer);
 
-       return (void *)timer;
+       return itimer;
 }
 
 
@@ -146,14 +134,9 @@ void isci_event_timer_start(
  * @timer: This parameter specifies the timer to be stopped.
  *
  */
-void isci_event_timer_stop(
-       struct scic_sds_controller *controller,
-       void *timer)
+void isci_event_timer_stop(struct scic_sds_controller *controller, void *timer)
 {
-       struct isci_host *isci_host;
-
-       isci_host =
-               (struct isci_host *)sci_object_get_association(controller);
+       struct isci_host *isci_host = sci_object_get_association(controller);
 
        dev_dbg(&isci_host->pdev->dev,
                "%s: isci_host = %p, timer = %p\n",
@@ -162,6 +145,16 @@ void isci_event_timer_stop(
        isci_timer_stop((struct isci_timer *)timer);
 }
 
+void isci_event_timer_destroy(struct scic_sds_controller *scic, void *timer)
+{
+        struct isci_host *ihost = sci_object_get_association(scic);
+
+       dev_dbg(&ihost->pdev->dev, "%s: ihost = %p, timer = %p\n",
+                       __func__, ihost, timer);
+
+       isci_del_timer(ihost, timer);
+}
+
 /**
  * isci_event_controller_start_complete() - This user callback will inform the
  *    user that the controller has finished the start process. The associated
index 98526e9fb14a90221745b92908966f8f94dd50bb..fa2f6aa1093c001d2b139c4e4a71ebd5ae25aca6 100644 (file)
@@ -111,6 +111,9 @@ void isci_event_timer_stop(
        struct scic_sds_controller *controller,
        void *timer);
 
+
+void isci_event_timer_destroy(struct scic_sds_controller *scic, void *timer);
+
 /**
  * isci_event_controller_start_complete() - This user callback will inform the
  *    user that the controller has finished the start process.
index d8d6f67bd69c7cd92cb6cafecb0df3e7d20318e6..1bc91f2b4f93827c5f28d8f4341924ea0fab07a5 100644 (file)
@@ -349,9 +349,14 @@ void isci_host_deinit(struct isci_host *ihost)
        }
 
        set_bit(IHOST_STOP_PENDING, &ihost->flags);
+
+       spin_lock_irq(&ihost->scic_lock);
        scic_controller_stop(scic, SCIC_CONTROLLER_STOP_TIMEOUT);
+       spin_unlock_irq(&ihost->scic_lock);
+
        wait_for_stop(ihost);
        scic_controller_reset(scic);
+       isci_timer_list_destroy(ihost);
 }
 
 static void __iomem *scu_base(struct isci_host *isci_host)
@@ -370,8 +375,6 @@ static void __iomem *smu_base(struct isci_host *isci_host)
        return pcim_iomap_table(pdev)[SCI_SMU_BAR * 2] + SCI_SMU_BAR_SIZE * id;
 }
 
-#define SCI_MAX_TIMER_COUNT 25
-
 int isci_host_init(struct isci_host *isci_host)
 {
        int err = 0;
@@ -382,11 +385,7 @@ int isci_host_init(struct isci_host *isci_host)
        union scic_oem_parameters scic_oem_params;
        union scic_user_parameters scic_user_params;
 
-       INIT_LIST_HEAD(&isci_host->timer_list_struct.timers);
-       isci_timer_list_construct(
-               &isci_host->timer_list_struct,
-               SCI_MAX_TIMER_COUNT
-               );
+       isci_timer_list_construct(isci_host);
 
        controller = scic_controller_alloc(&isci_host->pdev->dev);
 
@@ -473,7 +472,17 @@ int isci_host_init(struct isci_host *isci_host)
                }
        }
 
+       tasklet_init(&isci_host->completion_tasklet,
+                    isci_host_completion_routine, (unsigned long)isci_host);
+
+       INIT_LIST_HEAD(&(isci_host->mdl_struct_list));
+
+       INIT_LIST_HEAD(&isci_host->requests_to_complete);
+       INIT_LIST_HEAD(&isci_host->requests_to_abort);
+
+       spin_lock_irq(&isci_host->scic_lock);
        status = scic_controller_initialize(isci_host->core_controller);
+       spin_unlock_irq(&isci_host->scic_lock);
        if (status != SCI_SUCCESS) {
                dev_warn(&isci_host->pdev->dev,
                         "%s: scic_controller_initialize failed -"
@@ -482,17 +491,8 @@ int isci_host_init(struct isci_host *isci_host)
                return -ENODEV;
        }
 
-       tasklet_init(&isci_host->completion_tasklet,
-                    isci_host_completion_routine, (unsigned long)isci_host);
-
-       INIT_LIST_HEAD(&(isci_host->mdl_struct_list));
-
-       INIT_LIST_HEAD(&isci_host->requests_to_complete);
-       INIT_LIST_HEAD(&isci_host->requests_to_abort);
-
        /* populate mdl with dma memory. scu_mdl_allocate_coherent() */
        err = isci_host_mdl_allocate_coherent(isci_host);
-
        if (err)
                return err;
 
index b794dfd0819ee0400554d9177d10cf837c466d8e..ef3e7d1440b0245acddca036eefbc4b0db331cd4 100644 (file)
@@ -87,7 +87,7 @@ struct isci_host {
        union scic_oem_parameters oem_parameters;
 
        int id; /* unique within a given pci device */
-       struct isci_timer_list timer_list_struct;
+       struct list_head timers;
        void *core_ctrl_memory;
        struct dma_pool *dma_pool;
        unsigned int dma_pool_alloc_size;
index 6f98f6c74efbe0e634ffcb984d0989f516e95134..232125eab523c223962648cf83ec8c19eef77885 100644 (file)
@@ -475,14 +475,8 @@ int isci_task_execute_tmf(
        }
 
        /* Allocate the TMF timeout timer. */
-       tmf->timeout_timer = isci_timer_create(
-               &isci_host->timer_list_struct,
-               isci_host,
-               request,
-               isci_tmf_timeout_cb
-               );
-
        spin_lock_irqsave(&isci_host->scic_lock, flags);
+       tmf->timeout_timer = isci_timer_create(isci_host, request, isci_tmf_timeout_cb);
 
        /* Start the timer. */
        if (tmf->timeout_timer)
@@ -557,9 +551,7 @@ int isci_task_execute_tmf(
 
        /* Clean up the timer if needed. */
        if (tmf->timeout_timer) {
-               isci_timer_stop(tmf->timeout_timer);
-               isci_timer_free(&isci_host->timer_list_struct,
-                               tmf->timeout_timer);
+               isci_del_timer(isci_host, tmf->timeout_timer);
                tmf->timeout_timer = NULL;
        }
 
@@ -1468,10 +1460,7 @@ void isci_task_request_complete(
 
        /* Manage the timer if it is still running. */
        if (tmf->timeout_timer) {
-
-               isci_timer_stop(tmf->timeout_timer);
-               isci_timer_free(&isci_host->timer_list_struct,
-                               tmf->timeout_timer);
+               isci_del_timer(isci_host, tmf->timeout_timer);
                tmf->timeout_timer = NULL;
        }
 
index ca723089ee88bccf6a1d16549eaa5b8623594ba4..f33eff00dc0181005f1697ba63bf719dcb6a2cd2 100644 (file)
 #include "isci.h"
 #include "timers.h"
 
-
 /**
  * isci_timer_list_construct() - This method contrucst the SCI Timer List
  *    object used by the SCI Module class. The construction process involves
  *    creating isci_timer objects and adding them to the SCI Timer List
  *    object's list member. The number of isci_timer objects is determined by
  *    the timer_list_size parameter.
- * @isci_timer_list: This parameter points to the SCI Timer List object being
- *    constructed. The calling routine is responsible for allocating the memory
- *    for isci_timer_list and initializing the timer list_head member of
- *    isci_timer_list.
- * @timer_list_size: This parameter specifies the number of isci_timer objects
- *    contained by the SCI Timer List. which this timer is to be associated.
+ * @ihost: container of the timer list
  *
  * This method returns an error code indicating sucess or failure. The user
  * should check for possible memory allocation error return otherwise, a zero
  * indicates success.
  */
-int isci_timer_list_construct(
-       struct isci_timer_list *isci_timer_list,
-       int timer_list_size)
+int isci_timer_list_construct(struct isci_host *ihost)
 {
-       struct isci_timer *isci_timer;
-       int i;
-       int err = 0;
-
+       struct isci_timer *itimer;
+       int i, err = 0;
 
-       for (i = 0; i < timer_list_size; i++) {
-
-               isci_timer = kzalloc(sizeof(*isci_timer), GFP_KERNEL);
-
-               if (!isci_timer) {
+       INIT_LIST_HEAD(&ihost->timers);
+       for (i = 0; i < SCI_MAX_TIMER_COUNT; i++) {
+               itimer = devm_kzalloc(&ihost->pdev->dev, sizeof(*itimer), GFP_KERNEL);
 
+               if (!itimer) {
                        err = -ENOMEM;
                        break;
                }
-               isci_timer->used = 0;
-               isci_timer->stopped = 1;
-               isci_timer->parent = isci_timer_list;
-               list_add(&isci_timer->node, &isci_timer_list->timers);
+               init_timer(&itimer->timer);
+               itimer->used = 0;
+               itimer->stopped = 1;
+               list_add(&itimer->node, &ihost->timers);
        }
 
-       return 0;
-
+       return err;
 }
 
-
 /**
  * isci_timer_list_destroy() - This method destroys the SCI Timer List object
  *    used by the SCI Module class. The destruction  process involves freeing
  *    memory allocated for isci_timer objects on the SCI Timer List object's
  *    timers list_head member. If any isci_timer objects are mark as "in use",
  *    they are not freed and the function returns an error code of -EBUSY.
- * @isci_timer_list: This parameter points to the SCI Timer List object being
- *    destroyed.
- *
- * This method returns an error code indicating sucess or failure. The user
- * should check for possible -EBUSY error return, in the event of one or more
- * isci_timers still "in use", otherwise, a zero indicates success.
+ * @ihost: container of the list to be destroyed
  */
-int isci_timer_list_destroy(
-       struct isci_timer_list *isci_timer_list)
+void isci_timer_list_destroy(struct isci_host *ihost)
 {
-       struct isci_timer *timer, *tmp;
-
-       list_for_each_entry_safe(timer, tmp, &isci_timer_list->timers, node) {
-               isci_timer_free(isci_timer_list, timer);
-               list_del(&timer->node);
-               kfree(timer);
-       }
-       return 0;
-}
+       struct isci_timer *timer;
+       LIST_HEAD(list);
 
+       spin_lock_irq(&ihost->scic_lock);
+       list_splice_init(&ihost->timers, &list);
+       spin_unlock_irq(&ihost->scic_lock);
 
-
-static void isci_timer_restart(struct isci_timer *isci_timer)
-{
-       struct timer_list *timer =
-               &isci_timer->timer;
-       unsigned long timeout;
-
-       dev_dbg(&isci_timer->isci_host->pdev->dev,
-               "%s: isci_timer = %p\n", __func__, isci_timer);
-
-       isci_timer->restart = 0;
-       isci_timer->stopped = 0;
-       timeout = isci_timer->timeout_value;
-       timeout = (timeout * HZ) / 1000;
-       timeout = timeout ? timeout : 1;
-       mod_timer(timer, jiffies + timeout);
+       list_for_each_entry(timer, &list, node)
+               del_timer_sync(&timer->timer);
 }
 
 /**
@@ -169,7 +132,7 @@ static void isci_timer_restart(struct isci_timer *isci_timer)
 static void timer_function(unsigned long data)
 {
 
-       struct isci_timer *timer     = (struct isci_timer *)data;
+       struct isci_timer *timer = (struct isci_timer *)data;
        struct isci_host *isci_host = timer->isci_host;
        unsigned long flags;
 
@@ -185,89 +148,66 @@ static void timer_function(unsigned long data)
 
        if (!timer->stopped) {
                timer->stopped = 1;
-               timer->timer_callback(timer->cookie);
-
-               if (timer->restart)
-                       isci_timer_restart(timer);
+               timer->timer_callback(timer->cb_param);
        }
 
        spin_unlock_irqrestore(&isci_host->scic_lock, flags);
 }
 
 
-struct isci_timer *isci_timer_create(
-       struct isci_timer_list *isci_timer_list,
-       struct isci_host *isci_host,
-       void *cookie,
-       void (*timer_callback)(void *))
+struct isci_timer *isci_timer_create(struct isci_host *ihost, void *cb_param,
+                                    void (*timer_callback)(void *))
 {
-
        struct timer_list *timer;
        struct isci_timer *isci_timer;
-       struct list_head *timer_list =
-               &isci_timer_list->timers;
-       unsigned long flags;
+       struct list_head *list = &ihost->timers;
 
-       spin_lock_irqsave(&isci_host->scic_lock, flags);
+       WARN_ONCE(!spin_is_locked(&ihost->scic_lock),
+                 "%s: unlocked!\n", __func__);
 
-       if (list_empty(timer_list)) {
-               spin_unlock_irqrestore(&isci_host->scic_lock, flags);
+       if (WARN_ONCE(list_empty(list), "%s: timer pool empty\n", __func__))
                return NULL;
-       }
 
-       isci_timer = list_entry(timer_list->next, struct isci_timer, node);
+       isci_timer = list_entry(list->next, struct isci_timer, node);
 
-       if (isci_timer->used) {
-               spin_unlock_irqrestore(&isci_host->scic_lock, flags);
-               return NULL;
-       }
        isci_timer->used = 1;
        isci_timer->stopped = 1;
-       list_move_tail(&isci_timer->node, timer_list);
-
-       spin_unlock_irqrestore(&isci_host->scic_lock, flags);
+       /* FIXME: what!? we recycle the timer, rather than take it off
+        * the free list?
+        */
+       list_move_tail(&isci_timer->node, list);
 
        timer = &isci_timer->timer;
        timer->data = (unsigned long)isci_timer;
        timer->function = timer_function;
-       isci_timer->cookie = cookie;
+       isci_timer->cb_param = cb_param;
        isci_timer->timer_callback = timer_callback;
-       isci_timer->isci_host = isci_host;
+       isci_timer->isci_host = ihost;
 
-       dev_dbg(&isci_host->pdev->dev,
+       dev_dbg(&ihost->pdev->dev,
                "%s: isci_timer = %p\n", __func__, isci_timer);
 
        return isci_timer;
 }
 
-/**
- * isci_timer_free() - This method frees the isci_timer, marking it "free to
+/* isci_del_timer() - This method frees the isci_timer, marking it "free to
  *    use", then places its back at the head of the timers list for the SCI
  *    Timer List object specified.
- * @isci_timer_list: This parameter points to the SCI Timer List from which the
- *    timer is reserved.
- * @isci_timer: This parameter specifies the timer to be freed.
- *
  */
-void isci_timer_free(
-       struct isci_timer_list *isci_timer_list,
-       struct isci_timer *isci_timer)
+void isci_del_timer(struct isci_host *ihost, struct isci_timer *isci_timer)
 {
-       struct list_head *timer_list = &isci_timer_list->timers;
+       struct list_head *list = &ihost->timers;
+
+       WARN_ONCE(!spin_is_locked(&ihost->scic_lock),
+                 "%s unlocked!\n", __func__);
 
        dev_dbg(&isci_timer->isci_host->pdev->dev,
                "%s: isci_timer = %p\n", __func__, isci_timer);
 
-       if (list_empty(timer_list))
-               return;
-
        isci_timer->used = 0;
-       list_move(&isci_timer->node, timer_list);
-
-       if (!isci_timer->stopped) {
-               del_timer(&isci_timer->timer);
-               isci_timer->stopped = 1;
-       }
+       list_move(&isci_timer->node, list);
+       del_timer(&isci_timer->timer);
+       isci_timer->stopped = 1;
 }
 
 /**
@@ -278,26 +218,15 @@ void isci_timer_free(
  *    the associated callback function will be called.
  *
  */
-void isci_timer_start(
-       struct isci_timer *isci_timer,
-       unsigned long timeout)
+void isci_timer_start(struct isci_timer *isci_timer, unsigned long tmo)
 {
        struct timer_list *timer = &isci_timer->timer;
 
        dev_dbg(&isci_timer->isci_host->pdev->dev,
                "%s: isci_timer = %p\n", __func__, isci_timer);
 
-       isci_timer->timeout_value = timeout;
-       init_timer(timer);
-       timeout = (timeout * HZ) / 1000;
-       timeout = timeout ? timeout : 1;
-
-       timer->expires = jiffies + timeout;
-       timer->data = (unsigned long)isci_timer;
-       timer->function = timer_function;
        isci_timer->stopped = 0;
-       isci_timer->restart = 0;
-       add_timer(timer);
+       mod_timer(timer, jiffies + msecs_to_jiffies(tmo));
 }
 
 /**
@@ -310,10 +239,6 @@ void isci_timer_stop(struct isci_timer *isci_timer)
        dev_dbg(&isci_timer->isci_host->pdev->dev,
                "%s: isci_timer = %p\n", __func__, isci_timer);
 
-       if (isci_timer->stopped)
-               return;
-
        isci_timer->stopped = 1;
-
        del_timer(&isci_timer->timer);
 }
index ca5c2159a1c2ed1226a66d3697f4297eebd78793..8d8a892ad7f2af9c2d834dea1c57687ae6b47585 100644 (file)
 #include <linux/timer.h>
 #include <linux/types.h>
 
-/***************************************************
- * isci_timer
- ***************************************************
- */
-/**
- * struct isci_timer_list - This class is the container for all isci_timer
- *    objects
- *
- *
- */
-struct isci_timer_list {
-
-       struct list_head timers;
-};
+#define SCI_MAX_TIMER_COUNT 25
 
 /**
  * struct isci_timer - This class represents the timer object used by SCIC. It
- *    wraps the Linux timer_list object.
- *
+ *    wraps the Linux timer_list object, and (TODO) should be removed in favor
+ *    of a delayed-workqueue style interface with simpler locking
  *
  */
 struct isci_timer {
        int used;
        int stopped;
-       int restart;
-       int timeout_value;
-       void *cookie;
+       void *cb_param;
        void (*timer_callback)(void *);
        struct list_head node;
        struct timer_list timer;
-       struct isci_timer_list *parent;
        struct isci_host *isci_host;
 };
 
-#define to_isci_timer(t)       \
-       container_of(t, struct isci_timer, timer);
-
-int isci_timer_list_construct(
-       struct isci_timer_list *isci_timer_list,
-       int timer_list_size);
-
-
-int isci_timer_list_destroy(
-       struct isci_timer_list *isci_timer_list);
-
-struct isci_timer *isci_timer_create(
-       struct isci_timer_list *isci_timer_list,
-       struct isci_host *isci_host,
-       void *cookie,
-       void (*timer_callback)(void *));
-
-void isci_timer_free(
-       struct isci_timer_list *isci_timer_list,
-       struct isci_timer *isci_timer);
-
-void isci_timer_start(
-       struct isci_timer *isci_timer,
-       unsigned long timeout);
-
-void isci_timer_stop(
-       struct isci_timer *isci_timer);
-
+int isci_timer_list_construct(struct isci_host *ihost);
+void isci_timer_list_destroy(struct isci_host *ihost);
+struct isci_timer *isci_timer_create(struct isci_host *ihost, void *cb_param,
+                                    void (*timer_callback)(void *));
+void isci_del_timer(struct isci_host *ihost, struct isci_timer *itimer);
+void isci_timer_start(struct isci_timer *isci_timer, unsigned long timeout);
+void isci_timer_stop(struct isci_timer *isci_timer);
 
 #endif /* !defined (_SCI_TIMER_H_) */
-