Re: iSCSI target: Handling in-flight requests during ctld shutdown
Date: Fri, 31 Dec 2021 22:17:59 UTC
On 12/31/21 1:27 PM, Alexander Motin wrote: > On 31.12.2021 13:41, John Baldwin wrote: >> On 12/30/21 3:06 PM, Alexander Motin wrote: >>> No. cfiscsi_datamove_out() called before the new flag is set would >>> still try to send R2T over the dying connection to be aborted by the >>> cfiscsi_session_terminate_tasks() few milliseconds later. >>> cfiscsi_data_wait_abort() would only be needed if >>> cfiscsi_session_terminate_tasks() has already passed through the data >>> waiters list and waiting for the last tasks completion. >> >> So I think what I was missing is that I had assumed in the race case >> that the task was not visible when the NEXUS_I_T_RESET ran, but I think >> from re-reading now that the task has to have been in the lun's OOA >> queue as we don't permit queueing more tasks due to LUN_RESERVED being >> cleared, so I think that means that even in the race case the task >> has been aborted. Perhaps then the code in cfiscsi_datamove_out can >> just check CTL_FLAG_ABORT instead of cs_terminating? That would >> function similar to your proposed new flag I think assuming it is correct >> that the task for any I/O being passed to cfiscsi_datamove_out concurrent >> with cfiscsi_session_terminate_tasks must have been "visible" on the OAA >> queue and thus aborted by the handler? > > It was looking like a good idea for few seconds, since you right that > almost all commands should be visible via OAA queues and so should be > aborted by cfiscsi_session_terminate_tasks() at that point. But there > are few exceptions of commands that can be executed without LUNs > present, see CTL_CMD_FLAG_OK_ON_NO_LUN. All 3 of them are > CTL_FLAG_DATA_IN, so should not appear in cfiscsi_datamove_out(), but I > am still not sure it is very good, even though it may probably work. So my remaining question I guess is if I add a new 'cs->cs_terminating_tasks' or the like, how does cfiscsi_datamove_out ensure that no response is sent? The only thing I've seen so far is this code in cfiscsi_scsi_command_done: /* * Do not return status for aborted commands. * There are exceptions, but none supported by CTL yet. */ if (((io->io_hdr.flags & CTL_FLAG_ABORT) && (io->io_hdr.flags & CTL_FLAG_ABORT_STATUS) == 0) || (io->io_hdr.flags & CTL_FLAG_STATUS_SENT)) { ctl_free_io(io); icl_pdu_free(request); return; } Would you prefer checking cs_terminating_tasks in this function as well to avoid sending the peudo-aborted responses instead of forcing CTL_FLAG_ABORT on? -- John Baldwin