Re: [PATCH] cxl/port: Restart port enumeration when a sibling adds the dport first
Alison Schofield <[email protected]>
| Newsgroups | org.kernel.vger.linux-cxl |
|---|---|
| Message-ID | <[email protected]> |
On Mon, Jul 13, 2026 at 10:45:17AM +0800, Li Ming wrote:
> On 7/12/2026 9:28 AM, Alison Schofield wrote:
> > Endpoint probes can race while enumerating a shared switch. If a
> > sibling probe adds the dport first, the losing probe treats the
> > existing dport as an error 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.
>
> Hi Alison,
>
> Per commit log, I think the root cause is probe_dport() called in devm_cxl_create_port() twice, and the second time failed because the dport has been added.
>
> But devm_cxl_create_port() is protected by its parent port's device lock, probe_dport() is called only when a new port is created in devm_cxl_create_port(). If a port can be found, will return -EAGAIN directly in devm_cxl_create_port().
>
> So If probe_dport() was called twice, is it something wrong in find_cxl_port_by_uport()? Or I miss some details?
>
Hi Ming,
Thanks for the review and good catch! The commit log is vague and I've
reworded it for v2.
You're right about devm_cxl_create_port(): two probes both entering it
serialize on parent_port->dev and the loser then finds the port and
returns -EAGAIN without a second probe_dport(). find_cxl_port_by_uport()
is fine.
The -EBUSY comes from a different path. Two endpoints behind the same
switch race to add the shared dport and once one publishes the port the
other takes the port-found path instead of devm_cxl_create_port():
cxl_mem probe (endpoint 0) cxl_mem probe (endpoint 1)
-------------------------- --------------------------
find(uport) == NULL
add_port_attach_ep()
devm_cxl_create_port()
devm_cxl_add_port()
/* port published, */
/* dport not added yet */
find(uport) succeeds
find_or_add_dport()
probe_dport()
/* adds shared dport */
return -EAGAIN
guard(device)(&port->dev)
probe_dport()
dport_exists() == true
return -EBUSY
Before this patch, add_port_attach_ep() treated that -EBUSY as fatal and
dropped endpoint 0. Treating it like -EAGAIN restarts the walk, and that
finds the now present dport and continues.
Sending a v2 w commit msg update. Please take a look.
Thanks,
Alison
>
> Ming
>
> > Fixes: 4f06d81e7c6a ("cxl: Defer dport allocation for switch ports")
> > Signed-off-by: Alison Schofield <[email protected]>
> > ---
> > 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);
> > }
>
>