Re: [PATCH v4 06/14] cxl/region: Avoid duplicate call of cxl_find_decoder_early()

From: Dan Williams
Date: Thu Mar 06 2025 - 15:02:30 EST


Robert Richter wrote:
> Function cxl_find_decoder_early() is called twice, in
> alloc_region_ref() and cxl_rr_alloc_decoder(). Both functions are
> subsequently called from cxl_port_attach_region(). Make the decoder a
> function argument to both which avoids a duplicate call of
> cxl_find_decoder_early().
>
> Moving the call out of alloc_region_ref() also moves it out of the
> xa_for_each() loop in there. Now, cxld is determined no longer only
> for each auto-generated region, but now once for all regions
> regardless of auto-generated or not. This is fine as the cxld argument
> is needed for all regions in cxl_rr_alloc_decoder() and an error would
> be returned otherwise anyway. So it is better to determine the decoder
> in front of all this and fail early if missing instead of running
> through all that code with multiple calls of cxl_find_decoder_early().
>
> Signed-off-by: Robert Richter <rrichter@xxxxxxx>
> Reviewed-by: Gregory Price <gourry@xxxxxxxxxx>
> Tested-by: Gregory Price <gourry@xxxxxxxxxx>
> ---
> drivers/cxl/core/region.c | 31 +++++++++++++++----------------
> 1 file changed, 15 insertions(+), 16 deletions(-)

The change of order makes some sense, but now none of the naming makes
any sense.

So, if you pickup the cxl_port_pick_region_decoder() rename, that
implies an allocation. Then it follows that cxl_rr_alloc_decoder()
should be renamed to something like cxl_rr_assign_decoder(), because the
"alloc" was what cxl_region_find_decoder() was doing in that function.

That also would imply that cxl_rr_assign_decoder() cannot fail if all
the allocation work is done ahead of time.