From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f169.google.com (mail-qk1-f169.google.com [209.85.222.169]) (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 E455A2D0C79 for ; Sat, 28 Feb 2026 01:04:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772240643; cv=none; b=Lu2fgaJrvQkACCegvOl6e8PJJeC1r84ay4+sQ5TL9yNq2Yz/C+vGGr/colBXv+kFtJMU5OLytZtFPE2moME+6GqUlaRRaQq8re5w4FHI/k50zlSSxBLlhhNbxJelbbCvNC3BtI54dnQiWD45N20cGBiUxASkavGvOXvT08Wexyo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772240643; c=relaxed/simple; bh=hyovE6LN7Y7Or7QNp46+nybtUZ1XdvHs0V1eTBCR0Bo=; h=Message-ID:Date:MIME-Version:From:Subject:To:Cc:References: In-Reply-To:Content-Type; b=iN1QjjXQGQkjGyZL1S3UWmRhylToYjbZqrt/4TauHXQqEbnwwJBx2EeYRBB5RVxIqmAPJDvvEelHoeSLjJe8YdMwGuqZlG0AcUY75ToQaKn5LV4ZbzSujQzx8kqIzfapQ3mYHbKDvrdAk9u6nrihzUHDQryIVNhJFNyHZJ4ANAA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ae/v41EU; arc=none smtp.client-ip=209.85.222.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ae/v41EU" Received: by mail-qk1-f169.google.com with SMTP id af79cd13be357-8cb4097794dso247719485a.3 for ; Fri, 27 Feb 2026 17:04:00 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1772240639; x=1772845439; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id:from:to :cc:subject:date:message-id:reply-to; bh=sz+nw9RHvnXIemxs0AJJA/Skq4z5y0Q6hDr8HHtUbVk=; b=ae/v41EUDz4bxvgN0afk08mQgaEYQ38f7mxZqPumuANpasx2krSa895XeNCAUvXwhq xthBsHPGIrzqSfQm33Oi5eljxGef+VmTltDDHXNqnpQqA0mKIKgZ5oum/LmKfj4rJP8B 1Dj7OAPy4uKpjaVtc7/YkNWw2gY6RuMQm/sGlgFqWVzGhirShCb5T34m+AguyTKl7oxl kuHf23uxLhtPewDLZsTCjh7P/asqL66JCRLkM4qvyshVlSDy1NVNXCP3qDBeP2AghiP1 bmZTHVtXl6Awy2gAn0MHCXSxb+xbFUHVaiFUhEKt+DlLwsfulY8pNmTOae8mxVuXbhJI rKjQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772240639; x=1772845439; h=content-transfer-encoding:in-reply-to:content-language:references :cc:to:subject:from:user-agent:mime-version:date:message-id:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=sz+nw9RHvnXIemxs0AJJA/Skq4z5y0Q6hDr8HHtUbVk=; b=mFitUyOp+CIFp3Q6dV85LUHEQcjA0D5uiitb044pV6kIoS4QKBmvT6p9GO/CFDyMYQ nBqkhdSJhwEZYW+EKR+MY35BLu92y90d6K9C+i/4+ZFF4DS9z+D/EijPuM4eED8JtQvd yRZrih/+/7Z0ELB7Gr7Xn5AIO3wwFg9P3Bs71r0eRdX9KCPq+K2PU2Y6y5XJDuYGjBWR Xa9AIDS6HaLgIFgojnyuk9WG1MI5ANtd4ytDYFj//C/DXdzsuTc2MpkAbqilNUvAKi3x SvRkO9sUElcnFiGqpp8tBWbd9zpRadnTlbEhtslj3lu2Css0puBpuqtgW9b8o6sD6pIY nkEQ== X-Forwarded-Encrypted: i=1; AJvYcCVkdoFsYLiaTzcSVHlHs/IWZyQMp0Dsq3SLwNQmFdxoF3qW+5VvEMD1RJ8dD0Fku12hQkDdQ5EEU2B86Js=@vger.kernel.org X-Gm-Message-State: AOJu0Yy0059HmbjedNDtS98P2HbpMdWlLk2wGDV9mOfHXTbVd/DF84Zs y2HvkPE/0ft8G3e510CgBj9+s2FnjrSTJqVgdc2Uxd3B2PTNJTPROE+4 X-Gm-Gg: ATEYQzwGmhuSkyfZu7odSZJgUVilz9FX4koah0AcklIzDKfwQRskdMZ93ox4Z1xgAvH OEogWdb2Fvr9fjyg1nMHGCveYEYCZZzpKR9RO13GkyoziSI+NUr0IzoL6XK/uCr2ezhljw/idyB AsN5Wi3SICt8YhT8TIHA6jyFCALDwD60FvI0hA48s9fFAMy0+5a7DyyFTfGXpUqIbe87uzdVHr0 Qe9cMJWAVMsUA0sZGc7LqB1HYm24jNH4MVL4tpuICBQ64VAiInmkRzIigHVhiam2iqLZArnyKNS RpCNtxP1ANbdPtg1iyL/3TlSQGVfarlpbCXSKSOs4iodxIo93HkLDlkkKYIZEE2gHzcgflbVTCJ zYEkxYRcu0d3Saz29vMkY1rWtwTI7M8lKfXkuHp6LZpna2iz2wWvZPEDpX+UjFcAg0S7q0EYsRp uJU7MRElfrO9cc0VRmrhCabSTOy7GzCDrX9uaLD9UCvoLi X-Received: by 2002:a05:620a:470b:b0:8cb:4ad6:6aa0 with SMTP id af79cd13be357-8cbc8e12bc5mr604670985a.68.1772240639428; Fri, 27 Feb 2026 17:03:59 -0800 (PST) Received: from [10.69.56.189] ([192.19.223.252]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-507449630a0sm53364841cf.6.2026.02.27.17.03.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 27 Feb 2026 17:03:58 -0800 (PST) Message-ID: Date: Fri, 27 Feb 2026 17:03:55 -0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: James Smart Subject: Re: [PATCH v3 13/21] nvme-fc: Use CCR to recover controller that hits an error To: Mohamed Khalfella , Justin Tee , Naresh Gottumukkala , Paul Ely , Chaitanya Kulkarni , Christoph Hellwig , Jens Axboe , Keith Busch , Sagi Grimberg , Hannes Reinecke , jsmart833426@gmail.com Cc: Aaron Dailey , Randy Jennings , Dhaval Giani , linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org References: <20260214042753.4073668-1-mkhalfella@purestorage.com> <20260214042753.4073668-14-mkhalfella@purestorage.com> Content-Language: en-US In-Reply-To: <20260214042753.4073668-14-mkhalfella@purestorage.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 2/13/2026 8:25 PM, Mohamed Khalfella wrote: > An alive nvme controller that hits an error now will move to FENCING > state instead of RESETTING state. ctrl->fencing_work attempts CCR to > terminate inflight IOs. Regardless of the success or failure of CCR > operation the controller is transitioned to RESETTING state to continue > error recovery process. > > Signed-off-by: Mohamed Khalfella > --- > drivers/nvme/host/fc.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c > index e6ffaa19aba4..6ebabfb7e76d 100644 > --- a/drivers/nvme/host/fc.c > +++ b/drivers/nvme/host/fc.c > @@ -166,6 +166,7 @@ struct nvme_fc_ctrl { > struct blk_mq_tag_set admin_tag_set; > struct blk_mq_tag_set tag_set; > > + struct work_struct fencing_work; > struct work_struct ioerr_work; > struct delayed_work connect_work; > > @@ -1868,6 +1869,24 @@ __nvme_fc_fcpop_chk_teardowns(struct nvme_fc_ctrl *ctrl, > } > } > > +static void nvme_fc_fencing_work(struct work_struct *work) > +{ > + struct nvme_fc_ctrl *fc_ctrl = > + container_of(work, struct nvme_fc_ctrl, fencing_work); > + struct nvme_ctrl *ctrl = &fc_ctrl->ctrl; > + unsigned long rem; > + > + rem = nvme_fence_ctrl(ctrl); > + if (rem) { > + dev_info(ctrl->device, > + "CCR failed, skipping time-based recovery\n"); > + } > + > + nvme_change_ctrl_state(ctrl, NVME_CTRL_FENCED); > + if (nvme_change_ctrl_state(ctrl, NVME_CTRL_RESETTING)) > + queue_work(nvme_reset_wq, &fc_ctrl->ioerr_work); catch the rework of prior patch > +} > + > static void > nvme_fc_ctrl_ioerr_work(struct work_struct *work) > { > @@ -1889,6 +1908,7 @@ nvme_fc_ctrl_ioerr_work(struct work_struct *work) > return; > } > > + flush_work(&ctrl->fencing_work); > nvme_fc_error_recovery(ctrl); > } > > @@ -1915,6 +1935,14 @@ static void nvme_fc_start_ioerr_recovery(struct nvme_fc_ctrl *ctrl, > { > enum nvme_ctrl_state state; > From prior patch - the CONNECTING logic should be here.... > + if (nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_FENCING)) { > + dev_warn(ctrl->ctrl.device, > + "NVME-FC{%d}: starting controller fencing %s\n", > + ctrl->cnum, errmsg); > + queue_work(nvme_wq, &ctrl->fencing_work); > + return; > + } > + > if (nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_RESETTING)) { > dev_warn(ctrl->ctrl.device, "NVME-FC{%d}: starting error recovery %s\n", > ctrl->cnum, errmsg); > @@ -3322,6 +3350,7 @@ nvme_fc_reset_ctrl_work(struct work_struct *work) > struct nvme_fc_ctrl *ctrl = > container_of(work, struct nvme_fc_ctrl, ctrl.reset_work); > > + flush_work(&ctrl->fencing_work); > nvme_stop_ctrl(&ctrl->ctrl); > > /* will block will waiting for io to terminate */ > @@ -3497,6 +3526,7 @@ nvme_fc_alloc_ctrl(struct device *dev, struct nvmf_ctrl_options *opts, > > INIT_WORK(&ctrl->ctrl.reset_work, nvme_fc_reset_ctrl_work); > INIT_DELAYED_WORK(&ctrl->connect_work, nvme_fc_connect_ctrl_work); > + INIT_WORK(&ctrl->fencing_work, nvme_fc_fencing_work); > INIT_WORK(&ctrl->ioerr_work, nvme_fc_ctrl_ioerr_work); > spin_lock_init(&ctrl->lock); > there is a little to be in sync with my comment on the prior patch, but otherwise what is here is fine. What bothers me in this process is - there are certainly conditions where there is not connectivity loss where FC can send things such as the ABTS or a Disconnect LS that can inform the controller to start terminating. Its odd that we skip this step and go directly to the CCR reset to terminate the controller. We should have been able to continue to send the things that start to directly tear down the controller which can be happening in parallel with the CCR. -- james