[PATCH v5 0/2] scsi: target: fix LUN_RESET hang on aborted iSCSI WRITEs

[Laurence Oberman <loberman@redhat.com>]

These two patches fix a LUN_RESET hang on iSCSI targets, and the double
completion that fixing it exposes.

A WRITE command that is still in TRANSPORT_WRITE_PENDING has never been
passed to the backend, so when a TMR aborts it the fabric is the one that
has to complete it. Patch 2 completes it at the one site where nothing
does today. Patch 1 makes a second completion of an already-aborted
command a no-op, which is what makes that safe -- and which also fixes a
pre-existing double completion that is not specific to iSCSI.

 - Patch 1 guards the CMD_T_ABORTED branch of target_cmd_interrupted()
   with a one-shot flag taken under t_state_lock. Without it, a second
   target_complete_cmd() on an aborted command re-queues an se_cmd->work
   that is already queued and re-runs transport_complete_callback(), which
   for COMPARE AND WRITE releases dev->caw_sem a second time.

 - Patch 2 is the hang this work started from. A WRITE that arrives with
   immediate data, aborted after that data has been received but before
   the first R2T is built, is never completed by anything: no R2T has been
   sent so the initiator cannot send more data, and no DataOut timer is
   armed so the ERL=0 timeout cannot fire either. core_tmr_lun_reset()
   then blocks for as long as the connection stays up.

Changes since v4:
 - Moved the one-shot guard from iSCSI into target_cmd_interrupted(), as
   Bart Van Assche asked for in review of v4, and to the design he
   proposed there. This replaces the iSCSI-private completion_claimed
   member and the two helpers, which are gone, and it covers sites the
   iSCSI version could not: it closes the window where CMD_T_ABORTED is
   set between a fabric check and target_cmd_interrupted(), it covers the
   deferred ICF_OOO_CMDSN path into target_execute_cmd(), and it applies
   to every fabric rather than iSCSI alone.
 - The guard is limited to the CMD_T_ABORTED branch. complete_all() on the
   CMD_T_STOP branch is idempotent, and a shared guard would suppress the
   queueing of target_abort_work if CMD_T_ABORTED were set after a first
   call took the CMD_T_STOP branch.
 - Dropped v4 patches 2 and 3, which claimed the completion at two further
   iSCSI sites. Patch 1 makes both unnecessary.
 - Patch 2's Fixes: tag now names commit 2c9fa49e100f ("scsi: target/core: Make ABORT and LUN RESET handling synchronous")
   rather than the TAS commit it cited in v4. The TAS commit left the site
   with nothing to complete the command; making the abort path wait on a
   fabric completion is what turned that into a hang. Both are in v5.0, so
   the stable range is unchanged, and the patch body describes both.
 - Patch 2 is otherwise the same change as v4 patch 1, without the claim.

v4: https://lore.kernel.org/linux-scsi/20261026150722.823941-1-loberman@redhat.com/
v3: https://lore.kernel.org/linux-scsi/20261026124344.745124-1-loberman@redhat.com/
v2: https://lore.kernel.org/linux-scsi/20261016175351.126373-1-loberman@redhat.com/
v1: https://lore.kernel.org/linux-scsi/20261014143932.39453-1-loberman@redhat.com/

How this was found:

The hang was reported against an Oracle RAC cluster on two-session iSCSI
LIO targets, where one node's LUN_RESET aborts WRITEs belonging to the
other node's session. It was not reproducible in a lab, so the diagnosis
came from instrumented kernels in the reporter's environment, logging
every site that completes or waits on an aborted command.

Across two hosts every stall had the identical signature given in patch 2,
and the stall only ever ended when the initiator gave up and dropped the
connection. 217 of 222 LUN_RESET drains completed in under a second; the
rest took 46 to 54 seconds, which is where the hung-task panics came from.
One 53.7 second stall recovered on its own and the host ran for a further
19 hours.

An earlier attempt at this fix completed the command from
core_tmr_drain_state_list() instead. That is not safe: at that point the
initiator may still be sending DataOut for the command, and the result was
a double completion -- re-queued se_cmd->work, list_del corruption and a
refcount underflow. That is the failure patch 1 now prevents outright, and
it is worth knowing that the same symptoms are reachable without any of
this series.

What is and is not tested:

Patch 2's change has run 300 LUN_RESET cycles with no hang, in two
independent runs on a 5.14-based distro kernel, one instrumented and one
with the tracing removed. The same reproducer previously failed at 95 and
196 cycles. In those kernels the double completion was prevented by the
iSCSI-private claim that v4 carried rather than by patch 1, so the hang
fix is validated but patch 1 is not: it is new code, and its design is
Bart's rather than mine. I would rather say that plainly than imply field
coverage it does not have.

The reporter's environment is a production cluster and the only place this
has ever reproduced, so I cannot offer an upstream -rc run from there. A
build of this series is going to them and I will report the result.

Laurence Oberman (2):
  scsi: target: Complete an aborted command at most once
  scsi: target: iscsi: Complete an aborted WRITE stuck before the first
    R2T

 drivers/target/iscsi/iscsi_target_erl1.c | 13 ++++++++-
 drivers/target/target_core_transport.c   | 37 +++++++++++++++++++-----
 include/target/target_core_base.h        |  1 +
 3 files changed, 42 insertions(+), 9 deletions(-)

-- 
2.43.0