Re: [PATCH v5 2/3] cxlflash: Superpipe support

[Michael Neuling <mikey@neuling.org>]

> Add superpipe supporting infrastructure to device driver for the IBM CXL
> Flash adapter. This patch allows userspace applications to take advantage
> of the accelerated I/O features that this adapter provides and bypass the
> traditional filesystem stack.
> 
> Signed-off-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
> Signed-off-by: Manoj N. Kumar <manoj@linux.vnet.ibm.com>

Thanks for incorporating my suggestions.  One minor issue below but if
you fix that and add the ioctl version advertising as we talked about
offline, I'm happy if you add my reviewed by.

Reviewed-by: Michael Neuling <mikey@neuling.org>

> +static struct ctx_info *create_context(struct cxlflash_cfg *cfg,
> +				       struct cxl_context *ctx, int ctxid,
> +				       int adap_fd, struct file *file,
> +				       u32 perms)
> +{
> +	struct device *dev = &cfg->dev->dev;
> +	struct afu *afu = cfg->afu;
> +	struct ctx_info *ctxi = NULL;
> +	struct llun_info **lli = NULL;
> +	struct sisl_rht_entry *rhte;
> +
> +	ctxi = kzalloc(sizeof(*ctxi), GFP_KERNEL);
> +	lli = kzalloc((MAX_RHT_PER_CONTEXT * sizeof(*lli)), GFP_KERNEL);
> +	if (unlikely(!ctxi || !lli)) {
> +		dev_err(dev, "%s: Unable to allocate context!\n", __func__);
> +		goto out;

If only one of these allocations fails you'll leak some memory.  I
suggest making this "goto err", remove the "out" label and make err
look like this:

err:
	if lli
		kfree(lli);
	if ctxi
		kfree(ctxi);
	return NULL;

> +	}
> +
> +	rhte = (struct sisl_rht_entry *)get_zeroed_page(GFP_KERNEL);
> +	if (unlikely(!rhte)) {
> +		dev_err(dev, "%s: Unable to allocate RHT!\n", __func__);
> +		goto err;
> +	}
> +
> +	ctxi->rht_lun = lli;
> +	ctxi->rht_start = rhte;
> +	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;
> +	mutex_init(&ctxi->mutex);
> +	INIT_LIST_HEAD(&ctxi->luns);
> +	INIT_LIST_HEAD(&ctxi->list); /* initialize for list_empty() */
> +
> +	atomic_inc(&cfg->num_user_contexts);
> +	mutex_lock(&ctxi->mutex);
> +out:
> +	return ctxi;
> +
> +err:
> +	kfree(lli);
> +	kfree(ctxi);
> +	ctxi = NULL;
> +	goto out;
> +}