[PATCH 0/2] scsi: target: iscsi: fix stalled aborted WRITE commands

[Laurence Oberman <loberman@redhat.com>]

These two patches apply on top of 
commit d5869dae5080 ("scsi: target: iscsi: Fix hang for aborted WRITE_PENDING commands").

A WRITE command that is still in TRANSPORT_WRITE_PENDING has never been
passed to the backend, so when a TMR aborts it the iSCSI fabric is the one
that has to complete it. That commit fixed the case where the final
DataOut PDU for an already-issued R2T arrives after the abort. Two more
paths can leave such a command with nothing to complete it:

 - Patch 1 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.

 - Patch 2 closes the equivalent window where a new R2T would have been
   sent for an already-aborted command.

Patch 1 also adds a one-shot completion claim. Completing the command in
iscsit_execute_cmd() introduces a second site which
iscsit_release_commands_from_conn() would also complete during connection
teardown, because t_state is still TRANSPORT_WRITE_PENDING there and
target_complete_cmd() on an aborted command records nothing that would say
otherwise. The claim makes sure only one of them does it.

How this was found and tested:

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 1,
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.

Both patches, backported to a 5.14-based distro kernel, have since run 300
reset cycles with no hang, in two independent runs: one instrumented and
one with the tracing removed. The same reproducer previously failed at 95
and 196 cycles. The code here is equivalent to what was tested; only
comments differ.

One thing to flag for review: patch 2's window was never observed. The
instrumented kernels recorded no completions at that site in either run,
and the reproducer never reached it. It is included because the same
reasoning applies there, but it has no field evidence behind it, and I am
happy to drop it or send it separately if you would prefer.

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. Mentioning it in case the same idea comes up in review.


Laurence Oberman (2):
  scsi: target: iscsi: Complete an aborted WRITE stuck before the first
    R2T
  scsi: target: iscsi: Complete an aborted WRITE when a new R2T would be
    sent

 drivers/target/iscsi/iscsi_target.c      | 31 ++++++++++++++--
 drivers/target/iscsi/iscsi_target_erl1.c | 10 +++++-
 drivers/target/iscsi/iscsi_target_util.c | 46 ++++++++++++++++++++++++
 drivers/target/iscsi/iscsi_target_util.h |  2 ++
 include/target/iscsi/iscsi_target_core.h |  5 +++
 5 files changed, 91 insertions(+), 3 deletions(-)

-- 
2.52.0