| Message ID | 1519683712-17186-1-git-send-email-ukrishn@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
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 */ > }; >
> On Mar 22, 2018, at 11:40 AM, Frederic Barrat <fbarrat@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@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 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 */ >> }; >
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(); + 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); 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 */ };