Re: [PATCH v2 12/38] cxlflash: Use IDR to manage adapter contexts

[Frederic Barrat <fbarrat@linux.vnet.ibm.com>]

Le 26/02/2018 à 23:21, Uma Krishnan a écrit :
> A range of PASIDs are used as identifiers for the adapter contexts. These
> contexts may be destroyed and created randomly. Use an IDR to keep track
> of contexts that are in use and assign a unique identifier to new ones.
> 
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
> Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
> ---
>   drivers/scsi/cxlflash/ocxl_hw.c | 20 ++++++++++++++++++--
>   drivers/scsi/cxlflash/ocxl_hw.h |  2 ++
>   2 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index d75b873..6472210 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -12,6 +12,8 @@
>    * 2 of the License, or (at your option) any later version.
>    */
> 
> +#include <linux/idr.h>
> +
>   #include <misc/ocxl.h>
> 
>   #include "backend.h"
> @@ -60,14 +62,25 @@ static void *ocxlflash_dev_context_init(struct pci_dev *pdev, void *afu_cookie)
>   	if (unlikely(!ctx)) {
>   		dev_err(dev, "%s: Context allocation failed\n", __func__);
>   		rc = -ENOMEM;
> -		goto err;
> +		goto err1;
> +	}
> +
> +	idr_preload(GFP_KERNEL);
> +	rc = idr_alloc(&afu->idr, ctx, 0, afu->max_pasid, GFP_NOWAIT);
> +	idr_preload_end();


I believe we can call idr_alloc(... GFP_KERNEL) directly in that
context now.


> +	if (unlikely(rc < 0)) {
> +		dev_err(dev, "%s: idr_alloc failed rc=%d\n", __func__, rc);
> +		goto err2;
>   	}
> 
> +	ctx->pe = rc;
>   	ctx->master = false;
>   	ctx->hw_afu = afu;
>   out:
>   	return ctx;
> -err:
> +err2:
> +	kfree(ctx);
> +err1:
>   	ctx = ERR_PTR(rc);
>   	goto out;
>   }
> @@ -86,6 +99,7 @@ static int ocxlflash_release_context(void *ctx_cookie)
>   	if (!ctx)
>   		goto out;
> 
> +	idr_remove(&ctx->hw_afu->idr, ctx->pe);
>   	kfree(ctx);
>   out:
>   	return rc;
> @@ -103,6 +117,7 @@ static void ocxlflash_destroy_afu(void *afu_cookie)
>   		return;
> 
>   	ocxlflash_release_context(afu->ocxl_ctx);
> +	idr_destroy(&afu->idr);
>   	kfree(afu);
>   }
> 
> @@ -237,6 +252,7 @@ static void *ocxlflash_create_afu(struct pci_dev *pdev)
>   		goto err1;
>   	}
> 
> +	idr_init(&afu->idr);

You initialize the IDR too late. ocxlflash_dev_context_init() was called 
just above and allocated a PE.

   Fred


>   	afu->ocxl_ctx = ctx;
>   out:
>   	return afu;
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.h b/drivers/scsi/cxlflash/ocxl_hw.h
> index de43c04..0381682 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.h
> +++ b/drivers/scsi/cxlflash/ocxl_hw.h
> @@ -26,10 +26,12 @@ struct ocxl_hw_afu {
>   	int afu_actag_base;		/* AFU acTag base */
>   	int afu_actag_enabled;		/* AFU acTag number enabled */
> 
> +	struct idr idr;			/* IDR to manage contexts */
>   	int max_pasid;			/* Maximum number of contexts */
>   };
> 
>   struct ocxlflash_context {
>   	struct ocxl_hw_afu *hw_afu;	/* HW AFU back pointer */
>   	bool master;			/* Whether this is a master context */
> +	int pe;				/* Process element */
>   };
>