> On Mar 22, 2018, at 11:40 AM, Frederic Barrat <fbarrat@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
>> Acked-by: Matthew R. Ochs <mrochs@xxxxxxxxxxxxxxxxxx>
>> ---
>> 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 */
>> };
>
