[PATCH] cxl/region: Add dev_dbg() detail on failure to allocate HPA space

[PATCH] cxl/region: Add dev_dbg() detail on failure to allocate HPA space

[alison.schofield]

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: failed to allocate HPA:-34 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>
---
 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..0e6a378f1913 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,
+			"failed to allocate HPA:%ld size:%#llx in %s %pr\n",
+			PTR_ERR(res), size, cxlrd->res->name, cxlrd->res);
 		return PTR_ERR(res);
 	}
 

base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
-- 
2.37.3

Re: [PATCH] cxl/region: Add dev_dbg() detail on failure to allocate HPA space

[Dave Jiang]

On 11/13/23 21:01, alison.schofield@intel.com 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: failed to allocate HPA:-34 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>
> ---
>  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..0e6a378f1913 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,
> +			"failed to allocate HPA:%ld size:%#llx in %s %pr\n",
> +			PTR_ERR(res), size, cxlrd->res->name, cxlrd->res);
>  		return PTR_ERR(res);
>  	}
>  
> 
> base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86

Re: [PATCH] cxl/region: Add dev_dbg() detail on failure to allocate HPA space

[Verma, Vishal L]

On Mon, 2023-11-13 at 20:01 -0800, alison.schofield@intel.com 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: failed to allocate HPA:-34 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>
> ---
>  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..0e6a378f1913 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,
> +                       "failed to allocate HPA:%ld size:%#llx in %s %pr\n",
> +                       PTR_ERR(res), size, cxlrd->res->name, cxlrd->res);

Very minor, but the format of this print can make it seem like we're
talking about HPA -34 instead of an error code of -34. Might be nice to
take the chance to reword it to something like:

  "HPA allocation error (%d) for size: %llx in %s ..."

Other than that, looks good,

Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

>                 return PTR_ERR(res);
>         }
>  
> 
> base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86

-- 

Re: [PATCH] cxl/region: Add dev_dbg() detail on failure to allocate HPA space

[Alison Schofield]

On Wed, Nov 15, 2023 at 10:20:12AM -0800, Vishal Verma wrote:
> On Mon, 2023-11-13 at 20:01 -0800, alison.schofield@intel.com 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: failed to allocate HPA:-34 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>
> > ---
> >  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..0e6a378f1913 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,
> > +                       "failed to allocate HPA:%ld size:%#llx in %s %pr\n",
> > +                       PTR_ERR(res), size, cxlrd->res->name, cxlrd->res);
> 
> Very minor, but the format of this print can make it seem like we're
> talking about HPA -34 instead of an error code of -34. Might be nice to
> take the chance to reword it to something like:
> 
>   "HPA allocation error (%d) for size: %llx in %s ..."

Yes - much clearer. Will do.
Thanks!


> 
> Other than that, looks good,
> 
> Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>
> 
> >                 return PTR_ERR(res);
> >         }
> >
> >
> > base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
> 
> --
>