Re: [PATCH v2 08/14] nvme: Implement cross-controller reset recovery

[Mohamed Khalfella]

On Wed 2026-02-04 02:10:48 +0100, Hannes Reinecke wrote:
> On 2/3/26 21:00, Mohamed Khalfella wrote:
> > On Tue 2026-02-03 06:19:51 +0100, Hannes Reinecke wrote:
> >> On 1/30/26 23:34, Mohamed Khalfella wrote:
> [ .. ]
> >>> +	timeout = nvme_fence_timeout_ms(ictrl);
> >>> +	dev_info(ictrl->device, "attempting CCR, timeout %lums\n", timeout);
> >>> +
> >>> +	now = jiffies;
> >>> +	deadline = now + msecs_to_jiffies(timeout);
> >>> +	while (time_before(now, deadline)) {
> >>> +		sctrl = nvme_find_ctrl_ccr(ictrl, min_cntlid);
> >>> +		if (!sctrl) {
> >>> +			/* CCR failed, switch to time-based recovery */
> >>> +			return deadline - now;
> >>> +		}
> >>> +
> >>> +		ret = nvme_issue_wait_ccr(sctrl, ictrl);
> >>> +		if (!ret) {
> >>> +			dev_info(ictrl->device, "CCR succeeded using %s\n",
> >>> +				 dev_name(sctrl->device));
> >>> +			nvme_put_ctrl_ccr(sctrl);
> >>> +			return 0;
> >>> +		}
> >>> +
> >>> +		/* CCR failed, try another path */
> >>> +		min_cntlid = sctrl->cntlid + 1;
> >>> +		nvme_put_ctrl_ccr(sctrl);
> >>> +		now = jiffies;
> >>> +	}
> >>
> >> That will spin until 'deadline' is reached if 'nvme_issue_wait_ccr()'
> >> returns an error. _And_ if the CCR itself runs into a timeout we would
> >> never have tried another path (which could have succeeded).
> > 
> > True. We can do one thing at a time in CCR time budget. Either wait for
> > CCR to succeed or give up early and try another path. It is a trade off.
> > 
> Yes. But I guess my point here is that we should differentiate between
> 'CCR failed to be sent' and 'CCR completed with error'.
> The logic above treats both the same.
> 
> >>
> >> I'd rather rework this loop to open-code 'issue_and_wait()' in the loop,
> >> and only switch to the next controller if the submission of CCR failed.
> >> Once that is done we can 'just' wait for completion, as a failure there
> >> will be after KATO timeout anyway and any subsequent CCR would be pointless.
> > 
> > If I understood this correctly then we will stick with the first sctrl
> > that accepts the CCR command. We wait for CCR to complete and give up on
> > fencing ictrl if CCR operation fails or times out. Did I get this correctly?
> > 
> Yes.
> If a CCR could be send but the controller failed to process it something
> very odd is ongoing, and it's extremely questionable whether a CCR to
> another controller would be succeeding. That's why I would switch to the
> next available controller if we could not _send_ the CCR, but would
> rather wait for KATO if CCR processing returned an error.
> 
> But the main point is that CCR is a way to _shorten_ the interval
> (until KATO timeout) until we can start retrying commands.
> If the controller ran into an error during CCR processing chances
> are that quite some time has elapsed already, and we might as well
> wait for KATO instead of retrying with yet another CCR.

Got it. I updated the code to do that.

> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke                  Kernel Storage Architect
> hare@xxxxxxx                                +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