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

[Richard Cheng]

On Fri, Jul 31, 2026 at 12:06:43PM +0800, Alison Schofield wrote:
> 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>

Hi Alison,

Good idea, I'll form them into an inline static function in v9.
Thanks for this.

Best regards,
Richard Cheng.