Re: [PATCH v8 1/3] cxl/region: Simplify poison_by_decoder() error handling

[Alison Schofield]

On Fri, Jul 31, 2026 at 02:49:00PM +0800, Richard Cheng wrote:
> "rc" carries both an error code and the loop control
> signal for device_for_each_child(), so returning it bare is misleading,
> the early guards mean "keep walking", not "no error". Zeroing "rc" to
> forgive an -EFAULT on a RAM partition adds to that by discarding what
> the device actually returned.
> 
> Return a literal 0 where the walk should continue, and test the
> forgiven case directly instead of rewriting "rc". No functional change.

I walked thru to confirm the no functional change.
The old rc=0 was never read again on either patch. Each return rc you
converted was reachable only with rc at its initializer.

One comment below wrt the EFAULT case -

> 
> Suggested-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> Signed-off-by: Richard Cheng <icheng@xxxxxxxxxx>
> ---
>  drivers/cxl/core/region.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b6..fabaad3469b1 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2950,14 +2950,14 @@ static int poison_by_decoder(struct device *dev, void *arg)
>  	struct cxl_dev_state *cxlds;
>  	struct cxl_memdev *cxlmd;
>  	u64 offset, length;
> -	int rc = 0;
> +	int rc;
>  
>  	if (!is_endpoint_decoder(dev))
> -		return rc;
> +		return 0;
>  
>  	cxled = to_cxl_endpoint_decoder(dev);
>  	if (!cxled->dpa_res)
> -		return rc;
> +		return 0;
>  
>  	cxlmd = cxled_to_memdev(cxled);
>  	cxlds = cxlmd->cxlds;
> @@ -2967,18 +2967,14 @@ static int poison_by_decoder(struct device *dev, void *arg)
>  		offset = cxled->dpa_res->start - cxled->skip;
>  		length = cxled->skip;
>  		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
> -		if (rc == -EFAULT && mode == CXL_PARTMODE_RAM)
> -			rc = 0;
> -		if (rc)
> +		if (rc && (rc != -EFAULT || mode != CXL_PARTMODE_RAM))
>  			return rc;

I notice that the rule "forgive -EFAULT on a RAM partition" is now spelled
two different ways within twenty lines of each other.

cxl_get_poison_unmapped() uses the positive form, while here we invert
it. Can you flip that or add a poison_efault_forgiven() helper so both
usages read the same?

Reviewed-by: Alison Schofield <alison.schofield@xxxxxxxxx>