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

[Uma Krishnan]

> On Mar 22, 2018, at 11:40 AM, Frederic Barrat <fbarrat at linux.vnet.ibm.com> wrote:
> 
> 
> 
> 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 at linux.vnet.ibm.com>
>> Acked-by: Matthew R. Ochs <mrochs at 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

Good Catch. I will fix this bug and send out a V3 soon. Thanks for the review !!

> 
>>  	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 */
>>  };
>