> On Mar 22, 2018, at 11:40 AM, Frederic Barrat <fbar...@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 <ukri...@linux.vnet.ibm.com>
>> Acked-by: Matthew R. Ochs <mro...@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 */
>>  };
> 

Reply via email to