mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/4] scsi: target: iscsi: fix stalled and doubly completed aborted WRITEs
@ 2026-10-26 12:43 Laurence Oberman
  2026-10-26 12:43 ` [PATCH v3 1/4] scsi: target: iscsi: Complete an aborted WRITE stuck before the first R2T Laurence Oberman
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Laurence Oberman @ 2026-10-26 12:43 UTC (permalink / raw)
  To: mkp
  Cc: linux-scsi, target-devel, mlombard, michael.christie, bvanassche,
	linux-kernel

These four 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. Three more
paths can leave such a command with nothing to complete it, or let it be
completed twice:

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

 - Patches 3 and 4 fix two existing sites that complete an aborted
   WRITE_PENDING command without recording that they have done so, and
   which connection teardown can therefore complete a second time.

Patch 1 also adds a one-shot completion claim. target_complete_cmd() on a
command that already has CMD_T_ABORTED set goes into
target_cmd_interrupted(), which queues target_abort_work and returns
without setting CMD_T_COMPLETE and without changing t_state, so target
core records nothing that would stop a second completion. The command
stays in TRANSPORT_WRITE_PENDING and on conn_cmd_list until
target_abort_work reaches iscsit_aborted_task(), and
iscsit_release_commands_from_conn() completes anything it finds in
TRANSPORT_WRITE_PENDING. The claim is what makes exactly one site
complete the command.

Changes since v2:
 - New patch 3 routes the completion added by that commit through the
   claim, with Fixes: and Cc: stable. Raised in the review of v2, and by
   the Sashiko bot against both v1 and v2.
 - New patch 4 does the same for the DATAOUT_SEND_TO_TRANSPORT path into
   target_execute_cmd(), which has the same shape and is older.
 - Patch 1: reworded the completion_claimed comment. It said the flag is
   set once target_complete_cmd() has been called; it is set before the
   call, by whichever site takes the claim.
 - Patch 2: moved the "never observed in the field" paragraph below the
   --- line. Code unchanged.
 - Patches 1 and 2 are otherwise identical to v2.

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/

Two corrections to things I said during the v2 review:

I wrote that if the outstanding_r2ts read in patch 2 is stale low, "the
later completion in __iscsit_check_dataout_hdr() becomes a no-op". That
was wrong. In v2 that site calls target_complete_cmd() directly and never
looks at the claim, so the claim cannot make it a no-op. What actually
prevents a second completion there is that outstanding_r2ts is a u32, so
--cmd->outstanding_r2ts on a counter already at zero wraps to 0xffffffff
and fails the < 1 test. That is an accident of the existing code rather
than a guarantee, and it leaves the counter corrupted. Patch 3 removes
the need to rely on it. I have not tried to fix the underflow itself
here; say the word if you would like a patch for it.

I also said the pre-existing sites were best left to a follow-up. For the
record on the other half of that exchange, that commit is not confined
to 7.3/scsi-staging: it is in mainline, it carries CVE-2026-97951, and it
has been backported to 5.10.y, 5.15.y, 6.1.y, 6.6.y, 6.12.y, 6.18.y and
7.2.y. That is the reason patch 3 has Cc: stable -- and also a reason it
may be better taken as a standalone fix for those branches than as part
of this series, since none of them carry patch 1's helper. I will send
whichever form you prefer.

On fixing this in target core instead:

The root cause is that target_cmd_interrupted() queues target_abort_work
and records nothing under t_state_lock, so a test-and-set there would
cover every fabric rather than iSCSI alone, and would make the
completion_claimed field in this series unnecessary. I think that is the
better end state and I am willing to do it. I did not do it here for two
reasons. It changes the path that every fabric completes through, and I
have no way to test iSER, FC or SRP. And the field evidence behind this
series is evidence for an iSCSI-local change, not for a core one. I would
rather land the narrow fix for a hang that is being hit in production and
then propose the core cleanup on its own merits, in front of the people
who own that code.

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.

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.

What is and is not tested:

Patches 1 and 2, backported to a 5.14-based distro kernel, have run 300
LUN_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.

Only patch 1's window was ever observed. The instrumented kernels recorded
no completions at patch 2's site in either run, and patches 3 and 4 are
closed by inspection. The reporter's environment is a production cluster
and is the only place this has ever reproduced, so I cannot offer an
upstream -rc run or a MaxOutstandingR2T > 1 run from there. If that is a
blocker for patches 2 to 4, I would rather you took patch 1 on its own
than held the series.

Laurence Oberman (4):
  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
  scsi: target: iscsi: Claim the completion in
    __iscsit_check_dataout_hdr()
  scsi: target: iscsi: Claim the completion on the DataOut submit path

 drivers/target/iscsi/iscsi_target.c      | 44 +++++++++++++++++++----
 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 |  6 ++++
 5 files changed, 101 insertions(+), 7 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-26 12:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-26 12:43 [PATCH v3 0/4] scsi: target: iscsi: fix stalled and doubly completed aborted WRITEs Laurence Oberman
2026-10-26 12:43 ` [PATCH v3 1/4] scsi: target: iscsi: Complete an aborted WRITE stuck before the first R2T Laurence Oberman
2026-10-26 12:43 ` [PATCH v3 2/4] scsi: target: iscsi: Complete an aborted WRITE when a new R2T would be sent Laurence Oberman
2026-10-26 12:43 ` [PATCH v3 3/4] scsi: target: iscsi: Claim the completion in __iscsit_check_dataout_hdr() Laurence Oberman
2026-10-26 12:43 ` [PATCH v3 4/4] scsi: target: iscsi: Claim the completion on the DataOut submit path Laurence Oberman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®