Date: Fri, 31 Dec 2021 14:17:59 -0800 From: John Baldwin <jhb@FreeBSD.org> To: Alexander Motin <mav@FreeBSD.org> Cc: =?UTF-8?Q?Edward_Tomasz_Napiera=c5=82a?= <trasz@freebsd.org>, scsi@FreeBSD.org, Ken Merry <ken@freebsd.org> Subject: Re: iSCSI target: Handling in-flight requests during ctld shutdown Message-ID: <253e4d65-4c24-20be-480d-026c35d10ae5@FreeBSD.org> In-Reply-To: <1df3d9cf-6456-bcff-4fbe-c36136692efa@FreeBSD.org> References: <fd383f6f-5a19-e2bb-5383-e559271eb3cd@FreeBSD.org> <b6c090ac-6cb0-6173-422d-9aef0b37b8ee@FreeBSD.org> <42e175d9-1693-29e2-0b5b-3fa513aa2a2d@FreeBSD.org> <7f1a1a65-199d-858a-792c-42871d1df13e@FreeBSD.org> <68ad0ffd-79fb-973d-e5ba-92deee352d44@FreeBSD.org> <2ce80c64-6954-21d0-74eb-eeb88e289350@FreeBSD.org> <10687910-7500-008c-aed5-61f76ae90b3d@FreeBSD.org> <1df3d9cf-6456-bcff-4fbe-c36136692efa@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?253e4d65-4c24-20be-480d-026c35d10ae5>