[PATCH v5 08/16] nvme: Implement cross-controller reset recovery
Mohamed Khalfella
mkhalfella at purestorage.com
Fri Sep 4 16:04:48 PDT 2026
On Mon 2026-07-13 09:04:14 +0200, Hannes Reinecke wrote:
> On 7/12/26 4:23 AM, Mohamed Khalfella wrote:
> > A host that has more than one path connecting to an nvme subsystem
> > typically has an nvme controller associated with every path. This is
> > mostly applicable to nvmeof. If one path goes down, inflight IOs on that
> > path should not be retried immediately on another path because this
> > could lead to data corruption as described in TP4129. TP8028 defines
> > cross-controller reset mechanism that can be used by host to terminate
> > IOs on the failed path using one of the remaining healthy paths. Only
> > after IOs are terminated, or long enough time passes as defined by
> > TP4129, inflight IOs should be retried on another path. Implement core
> > cross-controller reset shared logic to be used by the transports.
> >
> > Signed-off-by: Mohamed Khalfella <mkhalfella at purestorage.com>
> > ---
> > drivers/nvme/host/constants.c | 1 +
> > drivers/nvme/host/core.c | 152 ++++++++++++++++++++++++++++++++++
> > drivers/nvme/host/nvme.h | 10 +++
> > 3 files changed, 163 insertions(+)
> >
> > diff --git a/drivers/nvme/host/constants.c b/drivers/nvme/host/constants.c
> > index dc90df9e13a2..f679efd5110e 100644
> > --- a/drivers/nvme/host/constants.c
> > +++ b/drivers/nvme/host/constants.c
> > @@ -46,6 +46,7 @@ static const char * const nvme_admin_ops[] = {
> > [nvme_admin_virtual_mgmt] = "Virtual Management",
> > [nvme_admin_nvme_mi_send] = "NVMe Send MI",
> > [nvme_admin_nvme_mi_recv] = "NVMe Receive MI",
> > + [nvme_admin_cross_ctrl_reset] = "Cross Controller Reset",
> > [nvme_admin_dbbuf] = "Doorbell Buffer Config",
> > [nvme_admin_format_nvm] = "Format NVM",
> > [nvme_admin_security_send] = "Security Send",
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index 1ef4dc38f755..a1deffc3cc00 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -571,6 +571,157 @@ void nvme_cancel_admin_tagset(struct nvme_ctrl *ctrl)
> > }
> > EXPORT_SYMBOL_GPL(nvme_cancel_admin_tagset);
> >
> > +static struct nvme_ctrl *nvme_find_ctrl_ccr(struct nvme_ctrl *ictrl,
> > + u32 min_cntlid)
> > +{
> > + struct nvme_subsystem *subsys = ictrl->subsys;
> > + struct nvme_ctrl *ctrl, *sctrl = NULL;
> > + unsigned long flags;
> > + int ccr_used;
> > +
> > + mutex_lock(&nvme_subsystems_lock);
> > + list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry) {
> > + if (ctrl->cntlid < min_cntlid)
> > + continue;
> > +
> > + spin_lock_irqsave(&ctrl->lock, flags);
> > + if (nvme_ctrl_state(ctrl) != NVME_CTRL_LIVE) {
> > + spin_unlock_irqrestore(&ctrl->lock, flags);
> > + continue;
> > + }
> > +
> > + ccr_used = atomic_inc_return(&ctrl->ccr_used);
> > + if (ccr_used > ctrl->ccrl) {
> > + atomic_dec(&ctrl->ccr_used);
> > + spin_unlock_irqrestore(&ctrl->lock, flags);
> > + continue;
> > + }
> > +
> > + /*
> > + * We got a good candidate source controller that is locked and
> > + * LIVE. However, no guarantee ctrl will not be deleted after
> > + * ctrl->lock is released. Get a ref of both ctrl and admin_q
> > + * so they do not disappear until we are done with them.
> > + */
> > + WARN_ON_ONCE(!blk_get_queue(ctrl->admin_q));
> > + nvme_get_ctrl(ctrl);
> > + spin_unlock_irqrestore(&ctrl->lock, flags);
> > + sctrl = ctrl;
> > + break;
> > + }
> > + mutex_unlock(&nvme_subsystems_lock);
> > + return sctrl;
> > +}
> > +
> > +static void nvme_put_ctrl_ccr(struct nvme_ctrl *sctrl)
> > +{
> > + atomic_dec(&sctrl->ccr_used);
> > + blk_put_queue(sctrl->admin_q);
> > + nvme_put_ctrl(sctrl);
> > +}
> > +
> > +static int nvme_issue_wait_ccr(struct nvme_ctrl *sctrl, struct nvme_ctrl *ictrl,
> > + unsigned long deadline)
> > +{
> > + struct nvme_ccr_entry ccr = { };
> > + union nvme_result res = { 0 };
> > + struct nvme_command c = { };
> > + unsigned long flags, now, tmo = 0;
> > + bool completed = false;
> > + int ret = 0;
> > + u32 result;
> > +
> > + init_completion(&ccr.complete);
> > + ccr.ictrl = ictrl;
> > +
> > + spin_lock_irqsave(&sctrl->lock, flags);
> > + list_add_tail(&ccr.list, &sctrl->ccr_list);
> > + spin_unlock_irqrestore(&sctrl->lock, flags);
> > +
> > + c.ccr.opcode = nvme_admin_cross_ctrl_reset;
> > + c.ccr.ciu = ictrl->ciu;
> > + c.ccr.icid = cpu_to_le16(ictrl->cntlid);
> > + c.ccr.cirn = cpu_to_le64(ictrl->cirn);
> > + ret = __nvme_submit_sync_cmd(sctrl->admin_q, &c, &res,
> > + NULL, 0, NVME_QID_ANY, 0);
> > + if (ret) {
> > + ret = -EIO;
> > + goto out;
> > + }
> > +
> > + result = le32_to_cpu(res.u32);
> > + if (result & 0x01) /* Immediate Reset Successful */
> > + goto out;
> > +
> Do we care about any other values of 'result' ?
No we do not. It just makes the code easier to read IMO.
>
> > + now = jiffies;
> > + if (time_before(now, deadline))
> > + tmo = min_t(unsigned long,
> > + secs_to_jiffies(ictrl->kato), deadline - now);
> > +
>
> Ideally we would check the request timeout for the CCR to not
> exceed 'deadline'.
> But as __nvme_submit_sync_cmd() doesn't allow to specify the timeout
> I guess it'll be good enough for now.
> (And if all goes to plan the CCR command should've been aborted in that
> case anyway).
Right, as a follow up work I would like to extend
__nvme_submit_sync_cmd() to take timeout, not in this patchset.
>
> > + if (!wait_for_completion_timeout(&ccr.complete, tmo)) {
> > + ret = -ETIMEDOUT;
> > + goto out;
> > + }
> > +
> > + completed = true;
> > +
> > +out:
> > + spin_lock_irqsave(&sctrl->lock, flags);
> > + list_del(&ccr.list);
> > + spin_unlock_irqrestore(&sctrl->lock, flags);
> > + if (completed) {
> > + if (ccr.ccrs == NVME_CCR_STATUS_SUCCESS)
> > + return 0;
> > + return -EREMOTEIO;
> > + }
> > + return ret;
> > +}
> > +
> > +unsigned long nvme_fence_ctrl(struct nvme_ctrl *ictrl)
> > +{
> > + unsigned long now, deadline, timeout;
> > + struct nvme_ctrl *sctrl;
> > + u32 min_cntlid = 0;
> > + int ret;
> > +
> > + timeout = nvme_fence_timeout_ms(ictrl);
> > + dev_info(ictrl->device, "attempting CCR, timeout %lums\n", timeout);
> > +
> > + now = jiffies;
> > + deadline = jiffies + msecs_to_jiffies(timeout);
> > + while (time_before(now, deadline)) {
> > + sctrl = nvme_find_ctrl_ccr(ictrl, min_cntlid);
> > + if (!sctrl) {
> > + dev_dbg(ictrl->device,
> > + "failed to find source controller\n");
> > + return deadline - now;
> > + }
> > +
> > + ret = nvme_issue_wait_ccr(sctrl, ictrl, deadline);
> > + if (!ret) {
> > + dev_info(ictrl->device, "CCR succeeded using %s\n",
> > + dev_name(sctrl->device));
> > + nvme_put_ctrl_ccr(sctrl);
> > + return 0;
> > + }
> > +
> > + /*
> > + * CCR command or CCR operation failed on this path.
> > + * Try another path as long as we have time.
> > + */
> > + dev_err(ictrl->device, "CCR failed using %s, ret = %d\n",
> > + dev_name(sctrl->device), ret);
> > + min_cntlid = sctrl->cntlid + 1;
> > + nvme_put_ctrl_ccr(sctrl);
> > + now = jiffies;
> > + }
> > +
> > + /* Fencing timed out call it done */
> > + dev_info(ictrl->device, "fencing timeout\n");
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(nvme_fence_ctrl);
> > +
> > bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
> > enum nvme_ctrl_state new_state)
> > {
> > @@ -5205,6 +5356,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> >
> > mutex_init(&ctrl->scan_lock);
> > INIT_LIST_HEAD(&ctrl->namespaces);
> > + INIT_LIST_HEAD(&ctrl->ccr_list);
> > xa_init(&ctrl->cels);
> > ctrl->dev = dev;
> > ctrl->ops = ops;
> > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> > index e558a19781bd..90b989302e21 100644
> > --- a/drivers/nvme/host/nvme.h
> > +++ b/drivers/nvme/host/nvme.h
> > @@ -333,6 +333,13 @@ enum nvme_ctrl_flags {
> > NVME_CTRL_FROZEN = 6,
> > };
> >
> > +struct nvme_ccr_entry {
> > + struct list_head list;
> > + struct completion complete;
> > + struct nvme_ctrl *ictrl;
> > + u8 ccrs;
> > +};
> > +
> > struct nvme_ctrl {
> > bool comp_seen;
> > bool identified;
> > @@ -350,6 +357,7 @@ struct nvme_ctrl {
> > struct blk_mq_tag_set *tagset;
> > struct blk_mq_tag_set *admin_tagset;
> > struct list_head namespaces;
> > + struct list_head ccr_list;
> > struct mutex namespaces_lock;
> > struct srcu_struct srcu;
> > struct device ctrl_device;
> > @@ -393,6 +401,7 @@ struct nvme_ctrl {
> > u16 sqsize;
> > u32 max_namespaces;
> > atomic_t abort_limit;
> > + atomic_t ccr_used;
> > u8 ccrl;
> > u8 vwc;
> > u32 vs;
> > @@ -887,6 +896,7 @@ blk_status_t nvme_host_path_error(struct request *req);
> > bool nvme_cancel_request(struct request *req, void *data);
> > void nvme_cancel_tagset(struct nvme_ctrl *ctrl);
> > void nvme_cancel_admin_tagset(struct nvme_ctrl *ctrl);
> > +unsigned long nvme_fence_ctrl(struct nvme_ctrl *ctrl);
> > bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
> > enum nvme_ctrl_state new_state);
> > int nvme_disable_ctrl(struct nvme_ctrl *ctrl, bool shutdown);
>
> Otherwise looks good.
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes Reinecke Kernel Storage Architect
> hare at suse.de +49 911 74053 688
> SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
> HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
More information about the Linux-nvme
mailing list