From nobody Fri Dec 31 22:17:59 2021 X-Original-To: scsi@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 3BA0D192E34D for ; Fri, 31 Dec 2021 22:18:02 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JQffK2wJrz4j4M; Fri, 31 Dec 2021 22:18:01 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from [IPV6:2601:648:8601:8b20:dc79:bf12:122a:616b] (unknown [IPv6:2601:648:8601:8b20:dc79:bf12:122a:616b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id ADD78F779; Fri, 31 Dec 2021 22:18:00 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Message-ID: <253e4d65-4c24-20be-480d-026c35d10ae5@FreeBSD.org> Date: Fri, 31 Dec 2021 14:17:59 -0800 List-Id: SCSI subsystem List-Archive: https://lists.freebsd.org/archives/freebsd-scsi List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-scsi@freebsd.org X-BeenThere: freebsd-scsi@freebsd.org MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Content-Language: en-US To: Alexander Motin Cc: =?UTF-8?Q?Edward_Tomasz_Napiera=c5=82a?= , scsi@FreeBSD.org, Ken Merry References: <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> From: John Baldwin Subject: Re: iSCSI target: Handling in-flight requests during ctld shutdown In-Reply-To: <1df3d9cf-6456-bcff-4fbe-c36136692efa@FreeBSD.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1640989082; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=WhTHZ8sgubJk/9wBmMLX9CgezVUy5uUPduw4A6U2Ek8=; b=hBCwFPRPYaChwSvHTGEGQg9U5rWu9ZmYUQCdWkUN7CDSVTc+co0k9j4k154HSpwkjCmuAe jy9Fam4YNSp2Cb3usD7vK66jhI/FxKmBzuPSqiEUiQ0h42L6ZwCKoG2V/jmVJBFwIJJA2h ZfJLXYEbqU6mdRf0heAFI6bqvzEdJR7JxLLd5sn9trcp95D/LLig2u7PFY1WgTb4HA4gqg ynLTeocpCtbfgSxyPU5tmjNdgQ3a/CkxYmjtYAXFbvbewwofQ+EoOKQ9UR0tkbOEVqYuZI LxvmhTnEx7e3hzd3m6Np6TYz40YS5SaE0oicFF5gfwy0BrIYchd4VmQX6H41ZA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1640989082; a=rsa-sha256; cv=none; b=Zq9r11wPxTYyvSdoN3TvLqPm4rNQvUFxTVTT6PW5o8tMSeeyK6fLlPRC7neWvx/f//KgQj o9k5/Oc0hzvyNbvWVa8MwHpBegO0xgS8AwpOgdtTHgdbPpXIBjUMk20MMlm5PWSj0+e3Lu HSJYUzZghFrF9AQhdbbc5FPHn9B9wKC2mR6pp9ARPfMZYC/jpv2nTBUloiFWeFp67sj6ph 5CRf2qzW71SL39+S9RI5Ilw5FcFmw1ugF0h7cFvCukEn5mRcpJuRwehrUJVEe4GMilGDLH i12acLK4KflDI3I9QFQJ7r3CAAaTZSriYb1AiJf5bKAxEcVBzWgGwcbsm6jclw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N 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