cxlflash: Split out context initialization
authorMatthew R. Ochs <mrochs@linux.vnet.ibm.com>
Fri, 4 Mar 2016 21:55:16 +0000 (15:55 -0600)
committerMartin K. Petersen <martin.petersen@oracle.com>
Wed, 9 Mar 2016 02:17:33 +0000 (21:17 -0500)
Presently, context information structures are allocated and
initialized in the same routine, create_context(). This imposes
an ordering restriction such that all pieces of information needed
to initialize a context must be known before the context is even
allocated.

This design point is not flexible when the order of context
creation needs to be modified. Specifically, this can lead to
problems when members of the context information structure are
a part of an ordering dependency (i.e. - the 'work' structure
embedded within the context).

To remedy, the allocation is left as-is, inside of the existing
create_context() routine and the initialization is transitioned
to a new void routine, init_context(). At the same time, in
anticipation of these routines not being called in sequence, a
state boolean is added to the context information structure to
track when the context has been initilized. The context teardown
routine, destroy_context(), is modified to support being called
with a non-initialized context.

Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
Reviewed-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
drivers/scsi/cxlflash/superpipe.c
drivers/scsi/cxlflash/superpipe.h

index f4020db..b30b362 100644 (file)
@@ -709,27 +709,32 @@ int cxlflash_disk_release(struct scsi_device *sdev,
  * @cfg:       Internal structure associated with the host.
  * @ctxi:      Context to release.
  *
- * Note that the rht_lun member of the context was cut from a single
- * allocation when the context was created and therefore does not need
- * to be explicitly freed. Also note that we conditionally check for the
- * existence of the context control map before clearing the RHT registers
- * and context capabilities because it is possible to destroy a context
- * while the context is in the error state (previous mapping was removed
- * [so we don't have to worry about clearing] and context is waiting for
- * a new mapping).
+ * This routine is safe to be called with a a non-initialized context
+ * and is tolerant of being called with the context's mutex held (it
+ * will be unlocked if necessary before freeing). Also note that the
+ * routine conditionally checks for the existence of the context control
+ * map before clearing the RHT registers and context capabilities because
+ * it is possible to destroy a context while the context is in the error
+ * state (previous mapping was removed [so there is no need to worry about
+ * clearing] and context is waiting for a new mapping).
  */
 static void destroy_context(struct cxlflash_cfg *cfg,
                            struct ctx_info *ctxi)
 {
        struct afu *afu = cfg->afu;
 
-       WARN_ON(!list_empty(&ctxi->luns));
+       if (ctxi->initialized) {
+               WARN_ON(!list_empty(&ctxi->luns));
 
-       /* Clear RHT registers and drop all capabilities for this context */
-       if (afu->afu_map && ctxi->ctrl_map) {
-               writeq_be(0, &ctxi->ctrl_map->rht_start);
-               writeq_be(0, &ctxi->ctrl_map->rht_cnt_id);
-               writeq_be(0, &ctxi->ctrl_map->ctx_cap);
+               /* Clear RHT registers and drop all capabilities for context */
+               if (afu->afu_map && ctxi->ctrl_map) {
+                       writeq_be(0, &ctxi->ctrl_map->rht_start);
+                       writeq_be(0, &ctxi->ctrl_map->rht_cnt_id);
+                       writeq_be(0, &ctxi->ctrl_map->ctx_cap);
+               }
+
+               if (mutex_is_locked(&ctxi->mutex))
+                       mutex_unlock(&ctxi->mutex);
        }
 
        /* Free memory associated with context */
@@ -742,23 +747,12 @@ static void destroy_context(struct cxlflash_cfg *cfg,
 /**
  * create_context() - allocates and initializes a context
  * @cfg:       Internal structure associated with the host.
- * @ctx:       Previously obtained CXL context reference.
- * @ctxid:     Previously obtained process element associated with CXL context.
- * @adap_fd:   Previously obtained adapter fd associated with CXL context.
- * @file:      Previously obtained file associated with CXL context.
- * @perms:     User-specified permissions.
- *
- * The context's mutex is locked when an allocated context is returned.
  *
  * Return: Allocated context on success, NULL on failure
  */
-static struct ctx_info *create_context(struct cxlflash_cfg *cfg,
-                                      struct cxl_context *ctx, int ctxid,
-                                      int adap_fd, struct file *file,
-                                      u32 perms)
+static struct ctx_info *create_context(struct cxlflash_cfg *cfg)
 {
        struct device *dev = &cfg->dev->dev;
-       struct afu *afu = cfg->afu;
        struct ctx_info *ctxi = NULL;
        struct llun_info **lli = NULL;
        u8 *ws = NULL;
@@ -781,28 +775,49 @@ static struct ctx_info *create_context(struct cxlflash_cfg *cfg,
        ctxi->rht_lun = lli;
        ctxi->rht_needs_ws = ws;
        ctxi->rht_start = rhte;
-       ctxi->rht_perms = perms;
+out:
+       return ctxi;
+
+err:
+       kfree(ws);
+       kfree(lli);
+       kfree(ctxi);
+       ctxi = NULL;
+       goto out;
+}
+
+/**
+ * init_context() - initializes a previously allocated context
+ * @ctxi:      Previously allocated context
+ * @cfg:       Internal structure associated with the host.
+ * @ctx:       Previously obtained CXL context reference.
+ * @ctxid:     Previously obtained process element associated with CXL context.
+ * @adap_fd:   Previously obtained adapter fd associated with CXL context.
+ * @file:      Previously obtained file associated with CXL context.
+ * @perms:     User-specified permissions.
+ *
+ * Upon return, the context is marked as initialized and the context's mutex
+ * is locked.
+ */
+static void init_context(struct ctx_info *ctxi, struct cxlflash_cfg *cfg,
+                        struct cxl_context *ctx, int ctxid, int adap_fd,
+                        struct file *file, u32 perms)
+{
+       struct afu *afu = cfg->afu;
 
+       ctxi->rht_perms = perms;
        ctxi->ctrl_map = &afu->afu_map->ctrls[ctxid].ctrl;
        ctxi->ctxid = ENCODE_CTXID(ctxi, ctxid);
        ctxi->lfd = adap_fd;
        ctxi->pid = current->tgid; /* tgid = pid */
        ctxi->ctx = ctx;
        ctxi->file = file;
+       ctxi->initialized = true;
        mutex_init(&ctxi->mutex);
        INIT_LIST_HEAD(&ctxi->luns);
        INIT_LIST_HEAD(&ctxi->list); /* initialize for list_empty() */
 
        mutex_lock(&ctxi->mutex);
-out:
-       return ctxi;
-
-err:
-       kfree(ws);
-       kfree(lli);
-       kfree(ctxi);
-       ctxi = NULL;
-       goto out;
 }
 
 /**
@@ -1396,13 +1411,16 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
        /* Translate read/write O_* flags from fcntl.h to AFU permission bits */
        perms = SISL_RHT_PERM(attach->hdr.flags + 1);
 
-       ctxi = create_context(cfg, ctx, ctxid, fd, file, perms);
+       ctxi = create_context(cfg);
        if (unlikely(!ctxi)) {
                dev_err(dev, "%s: Failed to create context! (%d)\n",
                        __func__, ctxid);
                goto err3;
        }
 
+       /* Context mutex is locked upon return */
+       init_context(ctxi, cfg, ctx, ctxid, fd, file, perms);
+
        work = &ctxi->work;
        work->num_interrupts = attach->num_interrupts;
        work->flags = CXL_START_WORK_NUM_IRQS;
index bede574..5f9a091 100644 (file)
@@ -102,6 +102,7 @@ struct ctx_info {
        u64 ctxid;
        int lfd;
        pid_t pid;
+       bool initialized;
        bool unavail;
        bool err_recovery_active;
        struct mutex mutex; /* Context protection */