From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-vs1-f53.google.com (mail-vs1-f53.google.com [209.85.217.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9A90513AD1C for ; Sat, 20 Dec 2025 01:21:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.217.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766193694; cv=none; b=cPwf/3Kr6JT/akIsrZrITrIYsZpb/31llKGNk2L9V3f4ZLj7gD/xG99jt2iNtJqiYHlbFoD5RpPSxhvLDutWT1qc84lzebwMvvnNhEC+nAUXuX1ygO104A9HiMKyRZJb+LDfNPdo2/CXCNDqV7yxqjJwLNkid18v6q/jkwIiZpw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766193694; c=relaxed/simple; bh=ZW4DvbOixkhWlewe7J2rTKJoM6gNu7V1eipU2FWZOQQ=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=SABta5GItqgmlL43vyTweeVejEVhhLUxfhoIxPXNUtDC/8B7kktFpFl+4NaGhh0eE5O1Zhga9fbHIRFx/JVqW2SDaGbYEEtE5cUlAh2LNOCMaSCkzBvNXv99z/8PC9EWdLMqyBThcR3WzY2lddujH7HaffbeNkn8s3Dz22jst/s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=purestorage.com; spf=fail smtp.mailfrom=purestorage.com; dkim=pass (2048-bit key) header.d=purestorage.com header.i=@purestorage.com header.b=RH47jNJc; arc=none smtp.client-ip=209.85.217.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=purestorage.com Authentication-Results: smtp.subspace.kernel.org; spf=fail smtp.mailfrom=purestorage.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=purestorage.com header.i=@purestorage.com header.b="RH47jNJc" Received: by mail-vs1-f53.google.com with SMTP id ada2fe7eead31-5dbe6be1ac5so845336137.1 for ; Fri, 19 Dec 2025 17:21:31 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=purestorage.com; s=google2022; t=1766193690; x=1766798490; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=7PrJnszBLojqKrV78aw+xEkBElYTLkB4kwu8K7rO+Uo=; b=RH47jNJcNHnc4bjtffcoKSuSKl1dIgskv/NmCIxFEMXZQFoQLRxSEy+nc1GVptGzmd hx7uP89l1Sq5n9XzbUfWvyN7XHEsXsNqrNK0stj+z7BK+ieEDvq8oJHgvMw1uw4X1PDl Me/Mycv6LziJnpHtq1Fm6o32GgJR7RCgSqbpn/KvPbMS+RvTa7wXJvRT1J9pBtCdiU4R gXvLZ+OdTHHPSAIooqH76Ay3/6ajQ3wVjlMgaX9uloQxsyFHAiaFmxP9hyXm9avOMRCP LrCyBd2rLHWyiTbuFGoep33CnaSVIdBMSjIEcMRpu5ZKPxhaz68rH9YyhivF4V5NTVKx QLHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1766193690; x=1766798490; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=7PrJnszBLojqKrV78aw+xEkBElYTLkB4kwu8K7rO+Uo=; b=BM3ItmP2iHVp+3vj/1I5+dOq459Yk3aMzg4ozHJF+hIXl3ZmoVM5sbovSFKnMyeCzm /+RueAvjSFgTdrvV7yxLMqAM3gvoAgKwfZGWp0KIMy8aqF4rHe9v2gwe41CasoAVMFIl +8Tc4GqBN6Iexk2XmP+87TxdoscJz+k32/2NDBKuRbjeCHyRqp1XEIstuv5gxz5/iy5H GI/2XPSEnYwkM1f4uVQdKsli+L1iDBCz8SaQ6Rsx6cHYYqN2AOy03D2EXl2pxZ/ulTTT 92t5V+P54dvsq0jIJQyRNtTVM2S4ATPX22ZYHxbehJhT9gwWvIrTgFk9xJO10I3VLQQh aQAw== X-Forwarded-Encrypted: i=1; AJvYcCVLWDLqUBDB/w5oellslriIzbrokehDT1+dVzF9wggYflODlp2bydnuBepLdcJRliL+AXW0lKIPFgN1V4A=@vger.kernel.org X-Gm-Message-State: AOJu0YzI7QD8AVMtmc3O+6H93tzKjrkRgJAPTIm5AWfXkFZV1xAgpwhR /TSmjSPAT+QUxZp2M28yVm0QO0UaeX2uk7zOgbN/9QAzXSdBzuNt5F8CfakCF/JcXgG697xvsuL XcYqv90nx4lcdO3LrhUYEZxo0gt1O5f0S6UWDT6IkOw== X-Gm-Gg: AY/fxX64iiO5Q9xa4D+GZpLgl3m5hSl+gOO/MyObm+6lpJqLJP5PFYC5gRJtS3ykp2c bW58i149SguREz4bmDphCJwia95WZKY4bG3TlozyCeXzseyMqvkw9936aL2JFXPQ20EM9iopNIA TiRVnNADy9qSTIWB6KpQ3g0HVzmnZJyzUwmhb4U3VN5T4GzXkb0/tCeOKMNWKDW9AKio1/GPFxS Fi+lWtANrc11D8ssj7Ld1DNmuXxIi4HsWVGeDd6e6cp7VpqKodG/kmVgc6niJDEkXWLjE0= X-Google-Smtp-Source: AGHT+IEAohaMZzHDChaXAY4ZHYfg8fxcwnJwg/jG5OT7jaSYaIiqW15GXMiM17UYvMKtATUc7d/jsfdr7g5EDnq+h3g= X-Received: by 2002:a05:6102:3347:b0:5de:62f:65b3 with SMTP id ada2fe7eead31-5eb1a8530damr1291543137.39.1766193690469; Fri, 19 Dec 2025 17:21:30 -0800 (PST) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20251126021250.2583630-1-mkhalfella@purestorage.com> <20251126021250.2583630-14-mkhalfella@purestorage.com> In-Reply-To: <20251126021250.2583630-14-mkhalfella@purestorage.com> From: Randy Jennings Date: Fri, 19 Dec 2025 17:21:19 -0800 X-Gm-Features: AQt7F2oC8n1NhHo2f5aP9w7GI-EduP30-XHKUTZn6_ij-sxT15j46oBH4EqgRs0 Message-ID: Subject: Re: [RFC PATCH 13/14] nvme-fc: Use CCR to recover controller that hits an error To: Mohamed Khalfella Cc: Chaitanya Kulkarni , Christoph Hellwig , Jens Axboe , Keith Busch , Sagi Grimberg , Aaron Dailey , John Meneghini , Hannes Reinecke , linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Nov 25, 2025 at 6:13=E2=80=AFPM Mohamed Khalfella wrote: > > An alive nvme controller that hits an error will now move to RECOVERING > state instead of RESETTING state. In RECOVERING state, ctrl->err_work > will attempt to use cross-controller recovery to terminate inflight IOs > on the controller. If CCR succeeds, then switch to RESETTING state and > continue error recovery as usuall by tearing down the controller, and > attempting reconnect to target. If CCR fails, the behavior of recovery "usuall" -> "usual" "attempt reconnecting" -> "attempting to reconnect" it would read better with "the" added: "reconnect to the target" > depends on whether CQT is supported or not. If CQT is supported, switch > to time-based recovery by holding inflight IOs until it is safe for them > to be retried. If CQT is not supported proceed to retry requests > immediately, as the code currently does. > diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c > @@ -1862,11 +1862,48 @@ __nvme_fc_fcpop_chk_teardowns(struct nvme_fc_ctrl= *ctrl, > +static int nvme_fc_recover_ctrl(struct nvme_ctrl *ctrl) > + queue_delayed_work(nvme_reset_wq, &to_fc_ctrl(ctrl)->ioerr_work, = rem); Just like nvme_rdma_recover_ctrl, nvme_fc_recover_ctrl is exactly the same as nvme_tcp_recover_ctrl. Seems like a core.c function nvme_recover_ctrl could take a delayed work queue, unifying the code. > nvme_fc_ctrl_ioerr_work(struct work_struct *work) > { > + if (nvme_ctrl_state(&ctrl->ctrl) =3D=3D NVME_CTRL_RECOVERING) { > + if (nvme_fc_recover_ctrl(&ctrl->ctrl)) > + return; > + } > > nvme_fc_error_recovery(ctrl); Inside of nvme_fc_error_recovery(), we call nvme_stop_keep_alive(). The state of the controller should not be LIVE while waiting for recovery, so I do not think we will succeed in sending keep alives, but I think this should move to before (or inside of) nvme_fc_recover_ctrl(). You have replaced all the calls to nvme_fc_error_recovery() with nvme_fc_start_ioerr_recovery(), so that might be okay. Sincerely, Randy Jennings