| Message ID | 20231129003953.1252985-1-alison.schofield@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] cxl/region: Add dev_dbg() detail on failure to allocate HPA space | expand |
alison.schofield@ wrote: > From: Alison Schofield <alison.schofield@intel.com> > > When the region driver fails while allocating HPA space for a > new region it can be because the parent resource, the CXL Window, > has no more available space. > > In that case, the debug user sees this message: > cxl_core:alloc_hpa:555: cxl region2: failed to allocate HPA: -34 > > Expand the message like this: > cxl_core:alloc_hpa:555: cxl region8: HPA allocation error:-34 for size:0x20000000 in CXL Window 0 [mem 0xf010000000-0xf04fffffff flags 0x200] > > Now the debug user can examine /proc/iomem and consider actions > like removing other allocations in that space or reducing the > size of their region request. > > Suggested-by: Dan Williams <dan.j.williams@intel.com> > Signed-off-by: Alison Schofield <alison.schofield@intel.com> > Reviewed-by: Dave Jiang <dave.jiang@intel.com> > Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> > --- > > Changes in v2: > - Improve the message text (Vishal) > - Add reviewed by tags (DaveJ, Vishal) > - Link to v1: > https://lore.kernel.org/linux-cxl/20231114040147.1124696-1-alison.schofield@intel.com/ > > > drivers/cxl/core/region.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 56e575c79bb4..ec792af873c4 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -552,8 +552,9 @@ static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) > res = alloc_free_mem_region(cxlrd->res, size, SZ_256M, > dev_name(&cxlr->dev)); > if (IS_ERR(res)) { > - dev_dbg(&cxlr->dev, "failed to allocate HPA: %ld\n", > - PTR_ERR(res)); > + dev_dbg(&cxlr->dev, > + "HPA allocation error:%ld for size:%#llx in %s %pr\n", > + PTR_ERR(res), size, cxlrd->res->name, cxlrd->res); I notice that this did not pick up Vishal's suggested "error (%d) ..." style for conveying the error code. "HPA allocation error (%d) for size: %llx in %s ..." I prefer that style as well.
On Mon, Dec 04, 2023 at 05:46:26PM -0800, Dan Williams wrote: > alison.schofield@ wrote: > > From: Alison Schofield <alison.schofield@intel.com> snip > > > > > > Changes in v2: > > - Improve the message text (Vishal) > > - Add reviewed by tags (DaveJ, Vishal) > > - Link to v1: > > https://lore.kernel.org/linux-cxl/20231114040147.1124696-1-alison.schofield@intel.com/ > > > > > > drivers/cxl/core/region.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > > index 56e575c79bb4..ec792af873c4 100644 > > --- a/drivers/cxl/core/region.c > > +++ b/drivers/cxl/core/region.c > > @@ -552,8 +552,9 @@ static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) > > res = alloc_free_mem_region(cxlrd->res, size, SZ_256M, > > dev_name(&cxlr->dev)); > > if (IS_ERR(res)) { > > - dev_dbg(&cxlr->dev, "failed to allocate HPA: %ld\n", > > - PTR_ERR(res)); > > + dev_dbg(&cxlr->dev, > > + "HPA allocation error:%ld for size:%#llx in %s %pr\n", > > + PTR_ERR(res), size, cxlrd->res->name, cxlrd->res); > > I notice that this did not pick up Vishal's suggested "error (%d) ..." > style for conveying the error code. > > "HPA allocation error (%d) for size: %llx in %s ..." > > I prefer that style as well. I should have said something in the changelog. I rejected the parentheses based on this: "Printing numbers in parentheses (%d) adds no value and should be avoided." in Documentation/process/coding-style.rst Since it's a dev_dbg() msg I'll go ahead and add those parens. (But this is not a precedent for parens all over ;)) Alison
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 56e575c79bb4..ec792af873c4 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -552,8 +552,9 @@ static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size) res = alloc_free_mem_region(cxlrd->res, size, SZ_256M, dev_name(&cxlr->dev)); if (IS_ERR(res)) { - dev_dbg(&cxlr->dev, "failed to allocate HPA: %ld\n", - PTR_ERR(res)); + dev_dbg(&cxlr->dev, + "HPA allocation error:%ld for size:%#llx in %s %pr\n", + PTR_ERR(res), size, cxlrd->res->name, cxlrd->res); return PTR_ERR(res); }