Re: [PATCH v2 12/38] cxlflash: Use IDR to manage adapter contexts
> On Mar 22, 2018, at 11:40 AM, Frederic Barrat> 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 >> Acked-by: Matthew R. Ochs >> --- >> 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 >> + >> #include >> #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(>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(>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(>idr); >> kfree(afu); >> } >> @@ -237,6 +252,7 @@ static void *ocxlflash_create_afu(struct pci_dev *pdev) >> goto err1; >> } >> +idr_init(>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 */ >> }; >
Re: [PATCH v2 12/38] cxlflash: Use IDR to manage adapter contexts
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 KrishnanAcked-by: Matthew R. Ochs --- 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 + #include #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(>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(>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(>idr); kfree(afu); } @@ -237,6 +252,7 @@ static void *ocxlflash_create_afu(struct pci_dev *pdev) goto err1; } + idr_init(>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 */ };
[PATCH v2 12/38] cxlflash: Use IDR to manage adapter contexts
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 KrishnanAcked-by: Matthew R. Ochs --- 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 + #include #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(>idr, ctx, 0, afu->max_pasid, GFP_NOWAIT); + idr_preload_end(); + 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(>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(>idr); kfree(afu); } @@ -237,6 +252,7 @@ static void *ocxlflash_create_afu(struct pci_dev *pdev) goto err1; } + idr_init(>idr); 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 */ }; -- 2.1.0