Re: [PATCH v2] cxl/port: Restart port enumeration when a sibling adds the dport first

在 2026/7/14 10:04, Alison Schofield 写道:
> Endpoint probes can race while enumerating a shared switch. If a
> sibling probe adds the dport first, the losing probe finds the dport
> already present, gets -EBUSY, and fails to enumerate the endpoint.
>
> Treat this race the same as the existing port-created case by
> restarting the port walk, allowing it to find the existing dport
> and continue enumeration.
>
> This race was discovered while testing a cxl_test mixed-granularity
> topology, where twelve endpoints behind shared switches are probed in
> parallel during module load.
>
> Fixes: 4f06d81e7c6a ("cxl: Defer dport allocation for switch ports")
> Signed-off-by: Alison Schofield <[email protected]>

Hi Alison,

After deep diving into this issue and using your cxl_test 
mixed-granularity topology patchset to reproduce it. What I observed is 
a little bit different from the case you mentioned in v1, I can provide 
more details about that.

I believe it always happens with the log like "cxl_mem memX: probe with 
driver cxl_mem failed with error -16".

It will happen with the multiply switch levels topology like:

                            CXL host bridge

                               (hb dport 1)

                                       |

                              CXL switch 1

                              (sw1 dport 1)

                                       |

                              CXL switch 2

               (sw2 dport 1)       (sw2 dport 2)

                         |                           |

                endpoint 1             endpoint 2

When endpoint 1 probing is creating cxl switch 1 port by calling 
add_port_attach_ep(), it finally calls devm_cxl_create_port() for port 
creation, devm_cxl_create_port() is responsible for port creation and 
the given dport creation, in above topology, they are "CXL switch 1" 
port and "sw1 dport 1" creation. devm_cxl_add_port() is used for a port 
creation and probe_dport() is for a dport creation in 
devm_cxl_create_port(). But there is a small windows between these two 
function which the parent port device lock cannot protect. When 
devm_cxl_add_port() done, the newly added port is visible in CXL 
subsystem, at this point, if endpoint 2 probing observes cxl switch 1 
port, it will call add_port_attach_ep() for "CXL switch 2" port 
creation, in add_port_attach_ep(), it will use probe_dport() to create 
parent port's dport, in this example, parent port is "cxl switch 1" 
port, parent port dport is "sw1 dport 1". This flow triggers the problem.

endpoint 1 probing  endpoint 2 probing

------------------------------------------------------------

add_port_attach_ep() for switch 1 port

   # hold parent port device lock(HB port device lock)

   devm_cxl_create_port() for switch 1 port

     devm_cxl_add_port()

     # switch 1 port can be observed

              add_port_attach_ep() for switch 2 port

                # hold parent port device lock(switch 1 port device lock)

                cxl_find_dport_by_dev() for parent dport of switch 2 
port   # parent dport(sw1 dport 1) has not been added yet.

                probe_dport() for parent dport(sw1 dport 1) addition

                devm_cxl_create_port() for switch 2 port

                # release parent port device lock(switch 1 port device lock)

     # hold the added port device lock(switch 1 port device lock)

     probe_dport() for the given dport(sw1 dport 1) addition

     fails because the given dport has been added by endpoint 2 probing


I am not sure if the find_or_add_dport() part you mentioned in v1 can 
also trigger the same problem, my understanding is that 
find_or_add_dport() needs to hold the same parent_port/host device lock. 
Anyway, we all know enumeration flow is complicated, maybe I ignore some 
cases.

Besides, seems like there is another enumeration issue can be found by 
using your cxl_test mixed-granularity topology patchset, because 
sometimes my environment still misses some memdevs without above log 
after loading cxl_test with your fixup. But it may be another issue, I 
am trying to figure it out.

For this case, feel free to add

Tested-by: Li Ming <[email protected]>

Reviewed-by: Li Ming <[email protected]>


Ming

> ---
>
> Changes in v2:
> - Commit log: note that race is btw different dport add paths for the
>    same shared dport (Ming)
>
>   drivers/cxl/core/port.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 1215ee4f4035..65f2d2f1eb00 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1749,8 +1749,8 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
>   					     parent_dport, uport_dev,
>   					     dport_dev);
>   		if (IS_ERR(dport)) {
> -			/* Port already exists, restart iteration */
> -			if (PTR_ERR(dport) == -EAGAIN)
> +			/* Port or dport already exists, restart iteration */
> +			if (PTR_ERR(dport) == -EAGAIN || PTR_ERR(dport) == -EBUSY)
>   				return 0;
>   			return PTR_ERR(dport);
>   		}
>
> base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482