From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f52.google.com (mail-qv1-f52.google.com [209.85.219.52]) (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 B8C1A25783C for ; Sat, 28 Feb 2026 01:10:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772241040; cv=none; b=aj/D9OZ++MDta1Ycwu+/HfZWat9043QtvG4fGGQnH0MeWk+e78a0wHji1KZY5suplKGQEEJDxhODu1VlQ7ZM2HkOta84knAOAky7uj+jC4aly/CPudpEGE+XXop0EEdI7veNIEjv9kXtuwCw7IrYeW9zsVEC2X/cydOiPUxusv8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772241040; c=relaxed/simple; bh=O7VywHIBoicwQxVs/ZAIIY8NdZ6RQjB6Oigl3Mld928=; h=Message-ID:Date:MIME-Version:From:Subject:To:Cc:References: In-Reply-To:Content-Type; b=rE8h53qNWvG1gJx/yx/WCQB6ykAIuYdrUXJUhlDMlv8GRjuLfkiCH7csACx9DjWwRyBSfB3E96ux53OFLW3UfPIz+FVDvuv3X+SVCgESzajHZjcjszhTM2TwwDhoVxOHugapWCscC6FMeH/0FTiUBZdX0TV+cho6GkvfQkcnVZ0= 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=TMpCGqLg; arc=none smtp.client-ip=209.85.219.52 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="TMpCGqLg" Received: by mail-qv1-f52.google.com with SMTP id 6a1803df08f44-896f82e5961so39257756d6.0 for ; Fri, 27 Feb 2026 17:10:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1772241038; x=1772845838; 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=havewdMOqWEAGK7hQIis2n7tDRk976XBnYGEqxZK/wY=; b=TMpCGqLgnYLx94P/y8XaXCVrYUAsnp+fWbOe0ON4u4dlY9AP7pAKVwomT1/iqALfWH AxLic8k0itoXCd165cMBsG+GG/x+zmRuZCA4ZSW2Alv+scO3wXfpz2ftL6qtpa5jGwVs Gzfy1/5rSU1cw+Mw1nsOaGHNMyIhEICn0UGBmhKdh5Sm98OyeHe5gnjsBML9Hv8efMud C21zraKymmL5v9Bjtmr8H+zg35jA3tpd/Sml5xaHRujC2AIKTXX2xdAej1w2UlSf+yWX 9TXFGb7j2Y7+lTfAnAyqTD49xRv7oizws9TW37KYdasjCniAPm93xNtVg9r8e6ICVR+6 ZQSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772241038; x=1772845838; 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=havewdMOqWEAGK7hQIis2n7tDRk976XBnYGEqxZK/wY=; b=PQ8vRskq1y8YhaC/JQaFJCBAXMs26sUSvH28j4cjPQiCPYDP4yUN9QHYgXCwDI7FxY UfLDhSrN3/HuCdvBw1vUa0apvJDpDYXGaytlMLJRqA+xxYZIlpgK5tWj0j7Dz5Dczdnt BhOFKANM3rQ78MI49nj51Z+1vK1ngtwC3W6bwyB0wUrJGHASz1EJrqOZI00VvncLzsiq xqzVctr+LzH9UXJfC7rpWuAJqH3YavSC8T5QYdswxUv8bvT7ap2X3AbJhecup2yLcY/C cMQ0XwFQoMr/wvrcJ9v2xOT24b1wvIU9xsSTxwwkuMOp6wQJhsJ3ybvqNa98h+0i6EmO FFhQ== X-Forwarded-Encrypted: i=1; AJvYcCV4G25S0oU+5xSSiUmD4/yIMKrQglQKIxhZGgnETyI8lerz5tfeiUHJ+O6wjtcLUfpSWBz/bCmwFbe/UnI=@vger.kernel.org X-Gm-Message-State: AOJu0YzgMoR6ma8rvrewT2ZoueD+ZE2RAIMZ9h+sVK34lHaCnUMufF6w EoBkyyTXd4xeJtKHdwzn2/DsSuRCo2Kpajcree0gInBWiqexvhxEUd9z X-Gm-Gg: ATEYQzx7mokwjX1XMZg1zFUCwiHYlI0p7ASSxM8+M8qVIQB5fc19cp+kH6MCOQ9G3SQ sOWtAetqruQS5/+kOS7N0QU6ONTrGiXAObTHswVtKPDA4aHqFMwtsrkjfbSiObuCSzFCRK1lRgu QLvfx+ZRqnl/rvTx22Nlq2z/TALjJGNPnGv+kANyr4AuA1LcitUS9uY24mpFKK89x0Ru36KkZkg dfQnE7Q+Bn3jYMDNuzSg8SHuV/wQfIXDWADicZYZDdJ5nOaa/+zTQxkBjFcI3p9++htBEZrvQVh rkmVxZ2GLRDEblx3ysxIrNDP6XdPUC6by/3NXcoHFE4q9vds4fvD3xJBCCv54z4j4grdJKqW/Xs Hv9RLsMjsWBCJpd/QGtejCCcY+iQyQ+rud5LlyLjDfU8/kBzd2slpBA5iwx9cq5LMqsP8Za0jN0 B0GfTjLLDRlIHMAoC6ZDKhIeibbwZiY2TvAdhy7gH3yRIXXX0zqBItA0U= X-Received: by 2002:a05:6214:411c:b0:897:1ff:a6c with SMTP id 6a1803df08f44-899d1d6c87fmr69692456d6.10.1772241037706; Fri, 27 Feb 2026 17:10:37 -0800 (PST) Received: from [10.69.56.189] ([192.19.223.252]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-5075feb4db9sm6645211cf.22.2026.02.27.17.10.35 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 27 Feb 2026 17:10:37 -0800 (PST) Message-ID: Date: Fri, 27 Feb 2026 17:10:34 -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 14/21] nvme-fc: Hold inflight requests while in FENCING state 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-15-mkhalfella@purestorage.com> Content-Language: en-US In-Reply-To: <20260214042753.4073668-15-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: > While in FENCING state, aborted inflight IOs should be held until fencing > is done. Update nvme_fc_fcpio_done() to not complete aborted requests or > requests with transport errors. These held requests will be canceled in > nvme_fc_delete_association() after fencing is done. nvme_fc_fcpio_done() > avoids racing with canceling aborted requests by making sure we complete > successful requests before waking up the waiting thread. > > Signed-off-by: Mohamed Khalfella > --- > drivers/nvme/host/fc.c | 61 +++++++++++++++++++++++++++++++++++------- > 1 file changed, 51 insertions(+), 10 deletions(-) > > diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c > index 6ebabfb7e76d..e605dd3f4a40 100644 > --- a/drivers/nvme/host/fc.c > +++ b/drivers/nvme/host/fc.c > @@ -172,7 +172,7 @@ struct nvme_fc_ctrl { > > struct kref ref; > unsigned long flags; > - u32 iocnt; > + atomic_t iocnt; > wait_queue_head_t ioabort_wait; > > struct nvme_fc_fcp_op aen_ops[NVME_NR_AEN_COMMANDS]; > @@ -1823,7 +1823,7 @@ __nvme_fc_abort_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_fcp_op *op) > atomic_set(&op->state, opstate); > else if (test_bit(FCCTRL_TERMIO, &ctrl->flags)) { > op->flags |= FCOP_FLAGS_TERMIO; > - ctrl->iocnt++; > + atomic_inc(&ctrl->iocnt); the atomic change is probably what corrects deadlocks you saw. > } > spin_unlock_irqrestore(&ctrl->lock, flags); > > @@ -1853,20 +1853,29 @@ nvme_fc_abort_aen_ops(struct nvme_fc_ctrl *ctrl) > } > > static inline void > +__nvme_fc_fcpop_count_one_down(struct nvme_fc_ctrl *ctrl) > +{ > + if (atomic_dec_return(&ctrl->iocnt) == 0) > + wake_up(&ctrl->ioabort_wait); > +} > + > +static inline bool > __nvme_fc_fcpop_chk_teardowns(struct nvme_fc_ctrl *ctrl, > struct nvme_fc_fcp_op *op, int opstate) > { > unsigned long flags; > + bool ret = false; > > if (opstate == FCPOP_STATE_ABORTED) { > spin_lock_irqsave(&ctrl->lock, flags); > if (test_bit(FCCTRL_TERMIO, &ctrl->flags) && > op->flags & FCOP_FLAGS_TERMIO) { > - if (!--ctrl->iocnt) > - wake_up(&ctrl->ioabort_wait); > + ret = true; > } > spin_unlock_irqrestore(&ctrl->lock, flags); > } > + > + return ret; > } > > static void nvme_fc_fencing_work(struct work_struct *work) > @@ -1969,7 +1978,8 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req) > struct nvme_command *sqe = &op->cmd_iu.sqe; > __le16 status = cpu_to_le16(NVME_SC_SUCCESS << 1); > union nvme_result result; > - bool terminate_assoc = true; > + bool op_term, terminate_assoc = true; > + enum nvme_ctrl_state state; > int opstate; > > /* > @@ -2102,16 +2112,38 @@ nvme_fc_fcpio_done(struct nvmefc_fcp_req *req) > done: > if (op->flags & FCOP_FLAGS_AEN) { > nvme_complete_async_event(&queue->ctrl->ctrl, status, &result); > - __nvme_fc_fcpop_chk_teardowns(ctrl, op, opstate); > + if (__nvme_fc_fcpop_chk_teardowns(ctrl, op, opstate)) > + __nvme_fc_fcpop_count_one_down(ctrl); > atomic_set(&op->state, FCPOP_STATE_IDLE); > op->flags = FCOP_FLAGS_AEN; /* clear other flags */ > nvme_fc_ctrl_put(ctrl); > goto check_error; > } > > - __nvme_fc_fcpop_chk_teardowns(ctrl, op, opstate); > + /* > + * We can not access op after the request is completed because it can > + * be reused immediately. At the same time we want to wakeup the thread > + * waiting for ongoing IOs _after_ requests are completed. This is > + * necessary because that thread will start canceling inflight IOs > + * and we want to avoid request completion racing with cancellation. > + */ > + op_term = __nvme_fc_fcpop_chk_teardowns(ctrl, op, opstate); > + > + /* > + * If we are going to terminate associations and the controller is > + * LIVE or FENCING, then do not complete this request now. Let error > + * recovery cancel this request when it is safe to do so. > + */ > + state = nvme_ctrl_state(&ctrl->ctrl); > + if (terminate_assoc && > + (state == NVME_CTRL_LIVE || state == NVME_CTRL_FENCING)) > + goto check_op_term; > + > if (!nvme_try_complete_req(rq, status, result)) > nvme_fc_complete_rq(rq); > +check_op_term: > + if (op_term) > + __nvme_fc_fcpop_count_one_down(ctrl); > > check_error: > if (terminate_assoc) > @@ -2750,7 +2782,8 @@ nvme_fc_start_fcp_op(struct nvme_fc_ctrl *ctrl, struct nvme_fc_queue *queue, > * cmd with the csn was supposed to arrive. > */ > opstate = atomic_xchg(&op->state, FCPOP_STATE_COMPLETE); > - __nvme_fc_fcpop_chk_teardowns(ctrl, op, opstate); > + if (__nvme_fc_fcpop_chk_teardowns(ctrl, op, opstate)) > + __nvme_fc_fcpop_count_one_down(ctrl); > > if (!(op->flags & FCOP_FLAGS_AEN)) { > nvme_fc_unmap_data(ctrl, op->rq, op); > @@ -3219,7 +3252,7 @@ nvme_fc_delete_association(struct nvme_fc_ctrl *ctrl) > > spin_lock_irqsave(&ctrl->lock, flags); > set_bit(FCCTRL_TERMIO, &ctrl->flags); > - ctrl->iocnt = 0; > + atomic_set(&ctrl->iocnt, 0); > spin_unlock_irqrestore(&ctrl->lock, flags); > > __nvme_fc_abort_outstanding_ios(ctrl, false); > @@ -3228,11 +3261,19 @@ nvme_fc_delete_association(struct nvme_fc_ctrl *ctrl) > nvme_fc_abort_aen_ops(ctrl); > > /* wait for all io that had to be aborted */ > + wait_event(ctrl->ioabort_wait, atomic_read(&ctrl->iocnt) == 0); > spin_lock_irq(&ctrl->lock); > - wait_event_lock_irq(ctrl->ioabort_wait, ctrl->iocnt == 0, ctrl->lock); > clear_bit(FCCTRL_TERMIO, &ctrl->flags); > spin_unlock_irq(&ctrl->lock); > > + /* > + * At this point all inflight requests have been successfully > + * aborted. Now it is safe to cancel all requests we decided > + * not to complete in nvme_fc_fcpio_done(). > + */ > + nvme_cancel_tagset(&ctrl->ctrl); > + nvme_cancel_admin_tagset(&ctrl->ctrl); > + > nvme_fc_term_aen_ops(ctrl); > > /* This looks good Signed-off-by: James Smart -- james