Re: [PATCH v2 RESEND] IB/isert: tear connections down from an ib_client on device removal

[Yehyeong Lee]

Thanks for the review.  All four are real.  v3 closes three of them and
most of the fourth; what is left of the fourth is below:

  https://lore.kernel.org/r/20261102061608.617186-1-yhlee@xxxxxxxxxxxxxxxxxx

The isert_free_np() race is a separate patch, 1/2, because it predates
this work.  Keeping isert_np->mutex over the release loop would have
brought back the deadlock commit 691b0480933f fixed, so 1/2 takes the
connections off isert_np one at a time, which empties isert_conn->node
before the mutex is dropped.

The isert_connected_handler() race was mine.  Both handlers used to be
reached only from isert_cma_handler(), which the CM serialises per cm_id,
so the two critical sections were safe until isert_remove_one() started
calling the teardown without that mutex.  2/2 now keeps isert_conn->mutex
across the state store and the list move.

ISER_CONN_INIT is marked by the teardown and released by
isert_connected_handler(), which refuses to promote a connection that is
already terminating.  Releasing it from the walk would race
isert_connect_error() for the single reference the connection holds.

For the racing connects I could reproduce the half where a connection the
walk has passed is promoted afterwards, and the same change closes it.  A
connect request arriving during the removal I could not reproduce on
soft-RoCE.  While isert_remove_one() is still running, the code refuses
it: isert_connect_request() now fills both lists in one isert_conn_mutex
section and returns -ENODEV when the device is already going away.

Your third ground stays open, and I should say so plainly rather than
claim the window is closed.  Once isert_remove_one() has returned the
isert_device is gone, so a connect request that arrives before
cma_remove_one() tears the listener down makes isert_device_get() create a
fresh one, with removing clear, and the connection stays in
ISER_CONN_INIT.  cma_process_remove() raises DEVICE_REMOVAL on that cm_id,
isert_teardown_conn() only marks the connection, and no further rdma_cm
event can reach it, because cma_send_device_removal_put() has set the id
to RDMA_CM_DEVICE_REMOVAL and cma_ib_handler() drops every non-timewait
event for an id that is not in RDMA_CM_CONNECT.
The cm_id is never destroyed, so cma_process_remove() blocks in
wait_for_completion(&cma_dev->comp).  The unpatched code blocks in the
same case, in wait_event_interruptible(), so this is not a regression and
not a memory-safety problem.  I have not reproduced it.  I would rather
leave it to a follow-up patch than widen this one.