1. Patchew
  2. linux
  3. [PATCH v3 00/21] TP8028 Rapid Path Failure Recovery
  4. View patch

[PATCH v3 09/21] nvme: Implement cross-controller reset completion

Mohamed Khalfella posted 21 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v3 09/21] nvme: Implement cross-controller reset completion
Posted by Mohamed Khalfella 1 month, 2 weeks ago 
An nvme source controller that issues CCR command expects to receive an
NVME_AER_NOTICE_CCR_COMPLETED when pending CCR succeeds or fails. Add
sctrl->ccr_work to read NVME_LOG_CCR logpage and wakeup any thread
waiting on CCR completion.

Signed-off-by: Mohamed Khalfella <mkhalfella@purestorage.com>
---
 drivers/nvme/host/core.c | 49 +++++++++++++++++++++++++++++++++++++++-
 drivers/nvme/host/nvme.h |  1 +
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 765b1524b3ed..a9fcde1b411b 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1916,7 +1916,8 @@ EXPORT_SYMBOL_GPL(nvme_set_queue_count);
 
 #define NVME_AEN_SUPPORTED \
 	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | \
-	 NVME_AEN_CFG_ANA_CHANGE | NVME_AEN_CFG_DISC_CHANGE)
+	 NVME_AEN_CFG_ANA_CHANGE | NVME_AEN_CFG_CCR_COMPLETE | \
+	 NVME_AEN_CFG_DISC_CHANGE)
 
 static void nvme_enable_aen(struct nvme_ctrl *ctrl)
 {
@@ -4880,6 +4881,47 @@ static void nvme_get_fw_slot_info(struct nvme_ctrl *ctrl)
 	kfree(log);
 }
 
+static void nvme_ccr_work(struct work_struct *work)
+{
+	struct nvme_ctrl *ctrl = container_of(work, struct nvme_ctrl, ccr_work);
+	struct nvme_ccr_entry *ccr;
+	struct nvme_ccr_log_entry *entry;
+	struct nvme_ccr_log *log;
+	unsigned long flags;
+	int ret, i;
+
+	log = kmalloc(sizeof(*log), GFP_KERNEL);
+	if (!log)
+		return;
+
+	ret = nvme_get_log(ctrl, 0, NVME_LOG_CCR, 0x01,
+			   0x00, log, sizeof(*log), 0);
+	if (ret)
+		goto out;
+
+	spin_lock_irqsave(&ctrl->lock, flags);
+	for (i = 0; i < le16_to_cpu(log->ne); i++) {
+		entry = &log->entries[i];
+		if (entry->ccrs == NVME_CCR_STATUS_IN_PROGRESS)
+			continue;
+
+		list_for_each_entry(ccr, &ctrl->ccr_list, list) {
+			struct nvme_ctrl *ictrl = ccr->ictrl;
+
+			if (ictrl->cntlid != le16_to_cpu(entry->icid) ||
+			    ictrl->ciu != entry->ciu)
+				continue;
+
+			/* Complete matching entry */
+			ccr->ccrs = entry->ccrs;
+			complete(&ccr->complete);
+		}
+	}
+	spin_unlock_irqrestore(&ctrl->lock, flags);
+out:
+	kfree(log);
+}
+
 static void nvme_fw_act_work(struct work_struct *work)
 {
 	struct nvme_ctrl *ctrl = container_of(work,
@@ -4956,6 +4998,9 @@ static bool nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
 	case NVME_AER_NOTICE_DISC_CHANGED:
 		ctrl->aen_result = result;
 		break;
+	case NVME_AER_NOTICE_CCR_COMPLETED:
+		queue_work(nvme_wq, &ctrl->ccr_work);
+		break;
 	default:
 		dev_warn(ctrl->device, "async event result %08x\n", result);
 	}
@@ -5145,6 +5190,7 @@ void nvme_stop_ctrl(struct nvme_ctrl *ctrl)
 	nvme_stop_failfast_work(ctrl);
 	flush_work(&ctrl->async_event_work);
 	cancel_work_sync(&ctrl->fw_act_work);
+	cancel_work_sync(&ctrl->ccr_work);
 	if (ctrl->ops->stop_ctrl)
 		ctrl->ops->stop_ctrl(ctrl);
 }
@@ -5268,6 +5314,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
 	ctrl->quirks = quirks;
 	ctrl->numa_node = NUMA_NO_NODE;
 	INIT_WORK(&ctrl->scan_work, nvme_scan_work);
+	INIT_WORK(&ctrl->ccr_work, nvme_ccr_work);
 	INIT_WORK(&ctrl->async_event_work, nvme_async_event_work);
 	INIT_WORK(&ctrl->fw_act_work, nvme_fw_act_work);
 	INIT_WORK(&ctrl->delete_work, nvme_delete_ctrl_work);
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index f3ab9411cac5..af6a4e83053e 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -365,6 +365,7 @@ struct nvme_ctrl {
 	struct nvme_effects_log *effects;
 	struct xarray cels;
 	struct work_struct scan_work;
+	struct work_struct ccr_work;
 	struct work_struct async_event_work;
 	struct delayed_work ka_work;
 	struct delayed_work failfast_work;
-- 
2.52.0
Re: [PATCH v3 09/21] nvme: Implement cross-controller reset completion
Posted by Hannes Reinecke 1 month, 2 weeks ago 
On 2/14/26 05:25, Mohamed Khalfella wrote:
> An nvme source controller that issues CCR command expects to receive an
> NVME_AER_NOTICE_CCR_COMPLETED when pending CCR succeeds or fails. Add
> sctrl->ccr_work to read NVME_LOG_CCR logpage and wakeup any thread
> waiting on CCR completion.
> 
> Signed-off-by: Mohamed Khalfella <mkhalfella@purestorage.com>
> ---
>   drivers/nvme/host/core.c | 49 +++++++++++++++++++++++++++++++++++++++-
>   drivers/nvme/host/nvme.h |  1 +
>   2 files changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 765b1524b3ed..a9fcde1b411b 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1916,7 +1916,8 @@ EXPORT_SYMBOL_GPL(nvme_set_queue_count);
>   
>   #define NVME_AEN_SUPPORTED \
>   	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | \
> -	 NVME_AEN_CFG_ANA_CHANGE | NVME_AEN_CFG_DISC_CHANGE)
> +	 NVME_AEN_CFG_ANA_CHANGE | NVME_AEN_CFG_CCR_COMPLETE | \
> +	 NVME_AEN_CFG_DISC_CHANGE)
>   
>   static void nvme_enable_aen(struct nvme_ctrl *ctrl)
>   {
> @@ -4880,6 +4881,47 @@ static void nvme_get_fw_slot_info(struct nvme_ctrl *ctrl)
>   	kfree(log);
>   }
>   
> +static void nvme_ccr_work(struct work_struct *work)
> +{
> +	struct nvme_ctrl *ctrl = container_of(work, struct nvme_ctrl, ccr_work);
> +	struct nvme_ccr_entry *ccr;
> +	struct nvme_ccr_log_entry *entry;
> +	struct nvme_ccr_log *log;
> +	unsigned long flags;
> +	int ret, i;
> +
> +	log = kmalloc(sizeof(*log), GFP_KERNEL);
> +	if (!log)
> +		return;
> +
> +	ret = nvme_get_log(ctrl, 0, NVME_LOG_CCR, 0x01,
> +			   0x00, log, sizeof(*log), 0);
> +	if (ret)
> +		goto out;
> +
> +	spin_lock_irqsave(&ctrl->lock, flags);
> +	for (i = 0; i < le16_to_cpu(log->ne); i++) {
> +		entry = &log->entries[i];
> +		if (entry->ccrs == NVME_CCR_STATUS_IN_PROGRESS)
> +			continue;
> +
> +		list_for_each_entry(ccr, &ctrl->ccr_list, list) {
> +			struct nvme_ctrl *ictrl = ccr->ictrl;
> +
> +			if (ictrl->cntlid != le16_to_cpu(entry->icid) ||
> +			    ictrl->ciu != entry->ciu)
> +				continue;
> +
> +			/* Complete matching entry */
> +			ccr->ccrs = entry->ccrs;
> +			complete(&ccr->complete);
> +		}
> +	}
> +	spin_unlock_irqrestore(&ctrl->lock, flags);
> +out:
> +	kfree(log);
> +}
> +
>   static void nvme_fw_act_work(struct work_struct *work)
>   {
>   	struct nvme_ctrl *ctrl = container_of(work,
> @@ -4956,6 +4998,9 @@ static bool nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
>   	case NVME_AER_NOTICE_DISC_CHANGED:
>   		ctrl->aen_result = result;
>   		break;
> +	case NVME_AER_NOTICE_CCR_COMPLETED:
> +		queue_work(nvme_wq, &ctrl->ccr_work);
> +		break;
>   	default:
>   		dev_warn(ctrl->device, "async event result %08x\n", result);
>   	}
> @@ -5145,6 +5190,7 @@ void nvme_stop_ctrl(struct nvme_ctrl *ctrl)
>   	nvme_stop_failfast_work(ctrl);
>   	flush_work(&ctrl->async_event_work);
>   	cancel_work_sync(&ctrl->fw_act_work);
> +	cancel_work_sync(&ctrl->ccr_work);
>   	if (ctrl->ops->stop_ctrl)
>   		ctrl->ops->stop_ctrl(ctrl);
>   }
> @@ -5268,6 +5314,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
>   	ctrl->quirks = quirks;
>   	ctrl->numa_node = NUMA_NO_NODE;
>   	INIT_WORK(&ctrl->scan_work, nvme_scan_work);
> +	INIT_WORK(&ctrl->ccr_work, nvme_ccr_work);
>   	INIT_WORK(&ctrl->async_event_work, nvme_async_event_work);
>   	INIT_WORK(&ctrl->fw_act_work, nvme_fw_act_work);
>   	INIT_WORK(&ctrl->delete_work, nvme_delete_ctrl_work);
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index f3ab9411cac5..af6a4e83053e 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -365,6 +365,7 @@ struct nvme_ctrl {
>   	struct nvme_effects_log *effects;
>   	struct xarray cels;
>   	struct work_struct scan_work;
> +	struct work_struct ccr_work;
>   	struct work_struct async_event_work;
>   	struct delayed_work ka_work;
>   	struct delayed_work failfast_work;

We really would need some indicator whether 'ccr' is supported at all.
Using the number of available CCR commands would be an option, if though
that would require us to keep two counters (one for the number of
possible outstanding CCRs, and one for the number of actual outstanding
CCRs.).

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@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
Re: [PATCH v3 09/21] nvme: Implement cross-controller reset completion
Posted by Mohamed Khalfella 1 month, 2 weeks ago 
On Mon 2026-02-16 13:43:51 +0100, Hannes Reinecke wrote:
> On 2/14/26 05:25, Mohamed Khalfella wrote:
> > An nvme source controller that issues CCR command expects to receive an
> > NVME_AER_NOTICE_CCR_COMPLETED when pending CCR succeeds or fails. Add
> > sctrl->ccr_work to read NVME_LOG_CCR logpage and wakeup any thread
> > waiting on CCR completion.
> > 
> > Signed-off-by: Mohamed Khalfella <mkhalfella@purestorage.com>
> > ---
> >   drivers/nvme/host/core.c | 49 +++++++++++++++++++++++++++++++++++++++-
> >   drivers/nvme/host/nvme.h |  1 +
> >   2 files changed, 49 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index 765b1524b3ed..a9fcde1b411b 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -1916,7 +1916,8 @@ EXPORT_SYMBOL_GPL(nvme_set_queue_count);
> >   
> >   #define NVME_AEN_SUPPORTED \
> >   	(NVME_AEN_CFG_NS_ATTR | NVME_AEN_CFG_FW_ACT | \
> > -	 NVME_AEN_CFG_ANA_CHANGE | NVME_AEN_CFG_DISC_CHANGE)
> > +	 NVME_AEN_CFG_ANA_CHANGE | NVME_AEN_CFG_CCR_COMPLETE | \
> > +	 NVME_AEN_CFG_DISC_CHANGE)
> >   
> >   static void nvme_enable_aen(struct nvme_ctrl *ctrl)
> >   {
> > @@ -4880,6 +4881,47 @@ static void nvme_get_fw_slot_info(struct nvme_ctrl *ctrl)
> >   	kfree(log);
> >   }
> >   
> > +static void nvme_ccr_work(struct work_struct *work)
> > +{
> > +	struct nvme_ctrl *ctrl = container_of(work, struct nvme_ctrl, ccr_work);
> > +	struct nvme_ccr_entry *ccr;
> > +	struct nvme_ccr_log_entry *entry;
> > +	struct nvme_ccr_log *log;
> > +	unsigned long flags;
> > +	int ret, i;
> > +
> > +	log = kmalloc(sizeof(*log), GFP_KERNEL);
> > +	if (!log)
> > +		return;
> > +
> > +	ret = nvme_get_log(ctrl, 0, NVME_LOG_CCR, 0x01,
> > +			   0x00, log, sizeof(*log), 0);
> > +	if (ret)
> > +		goto out;
> > +
> > +	spin_lock_irqsave(&ctrl->lock, flags);
> > +	for (i = 0; i < le16_to_cpu(log->ne); i++) {
> > +		entry = &log->entries[i];
> > +		if (entry->ccrs == NVME_CCR_STATUS_IN_PROGRESS)
> > +			continue;
> > +
> > +		list_for_each_entry(ccr, &ctrl->ccr_list, list) {
> > +			struct nvme_ctrl *ictrl = ccr->ictrl;
> > +
> > +			if (ictrl->cntlid != le16_to_cpu(entry->icid) ||
> > +			    ictrl->ciu != entry->ciu)
> > +				continue;
> > +
> > +			/* Complete matching entry */
> > +			ccr->ccrs = entry->ccrs;
> > +			complete(&ccr->complete);
> > +		}
> > +	}
> > +	spin_unlock_irqrestore(&ctrl->lock, flags);
> > +out:
> > +	kfree(log);
> > +}
> > +
> >   static void nvme_fw_act_work(struct work_struct *work)
> >   {
> >   	struct nvme_ctrl *ctrl = container_of(work,
> > @@ -4956,6 +4998,9 @@ static bool nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
> >   	case NVME_AER_NOTICE_DISC_CHANGED:
> >   		ctrl->aen_result = result;
> >   		break;
> > +	case NVME_AER_NOTICE_CCR_COMPLETED:
> > +		queue_work(nvme_wq, &ctrl->ccr_work);
> > +		break;
> >   	default:
> >   		dev_warn(ctrl->device, "async event result %08x\n", result);
> >   	}
> > @@ -5145,6 +5190,7 @@ void nvme_stop_ctrl(struct nvme_ctrl *ctrl)
> >   	nvme_stop_failfast_work(ctrl);
> >   	flush_work(&ctrl->async_event_work);
> >   	cancel_work_sync(&ctrl->fw_act_work);
> > +	cancel_work_sync(&ctrl->ccr_work);
> >   	if (ctrl->ops->stop_ctrl)
> >   		ctrl->ops->stop_ctrl(ctrl);
> >   }
> > @@ -5268,6 +5314,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
> >   	ctrl->quirks = quirks;
> >   	ctrl->numa_node = NUMA_NO_NODE;
> >   	INIT_WORK(&ctrl->scan_work, nvme_scan_work);
> > +	INIT_WORK(&ctrl->ccr_work, nvme_ccr_work);
> >   	INIT_WORK(&ctrl->async_event_work, nvme_async_event_work);
> >   	INIT_WORK(&ctrl->fw_act_work, nvme_fw_act_work);
> >   	INIT_WORK(&ctrl->delete_work, nvme_delete_ctrl_work);
> > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> > index f3ab9411cac5..af6a4e83053e 100644
> > --- a/drivers/nvme/host/nvme.h
> > +++ b/drivers/nvme/host/nvme.h
> > @@ -365,6 +365,7 @@ struct nvme_ctrl {
> >   	struct nvme_effects_log *effects;
> >   	struct xarray cels;
> >   	struct work_struct scan_work;
> > +	struct work_struct ccr_work;
> >   	struct work_struct async_event_work;
> >   	struct delayed_work ka_work;
> >   	struct delayed_work failfast_work;
> 
> We really would need some indicator whether 'ccr' is supported at all.

Why do we need this indicator, other than exporting it via sysfs?

> Using the number of available CCR commands would be an option, if though
> that would require us to keep two counters (one for the number of
> possible outstanding CCRs, and one for the number of actual outstanding
> CCRs.).

Like mentioned above ctrl->ccr_limit gives us the number of ccrs
available now. It is not 100% indicator if CCR is supported or not, but
it is enough to implement CCR. A second counter can help us skip trying
CCR if we know impacted controller does not support it.

Do you think it is worth it?

Iterating over controllers in the subsystem is not that bad IMO. This is
similar to the point raised by James Smart [1].

1- https://lore.kernel.org/all/05875e07-b908-425a-ba6f-5e060e03241e@gmail.com/
Re: [PATCH v3 09/21] nvme: Implement cross-controller reset completion
Posted by Hannes Reinecke 1 month, 1 week ago 
On 2/17/26 19:25, Mohamed Khalfella wrote:
> On Mon 2026-02-16 13:43:51 +0100, Hannes Reinecke wrote:
[ .. ]
>>
>> We really would need some indicator whether 'ccr' is supported at all.
> 
> Why do we need this indicator, other than exporting it via sysfs?
> 
To avoid false positives.

>> Using the number of available CCR commands would be an option, if though
>> that would require us to keep two counters (one for the number of
>> possible outstanding CCRs, and one for the number of actual outstanding
>> CCRs.).
> 
> Like mentioned above ctrl->ccr_limit gives us the number of ccrs
> available now. It is not 100% indicator if CCR is supported or not, but
> it is enough to implement CCR. A second counter can help us skip trying
> CCR if we know impacted controller does not support it.
> 
> Do you think it is worth it?
> 
Yes. The problem is that we want to get towards TP8028 compliance, which
forces us to wait for 2*KATO + CQT before requests on the failed patch
can be retried. That will cause a _noticeable_ stall on the application
side. And the only way to shorten that is CCR; once we get confirmation
from CCR we can start retrying immediately.
At the same time the current implementation only waits for 1*KATO before
retrying, so there will be regression if we switch to TP8028-compliant
KATO handling for systems not supporting CCR.

So we can (and should) use CCR as the determining factor whether we
want to switch to TP8028-compliant behaviour or stick with the original
implementation.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@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
Re: [PATCH v3 09/21] nvme: Implement cross-controller reset completion
Posted by Mohamed Khalfella 1 month, 1 week ago 
On Wed 2026-02-18 08:51:31 +0100, Hannes Reinecke wrote:
> On 2/17/26 19:25, Mohamed Khalfella wrote:
> > On Mon 2026-02-16 13:43:51 +0100, Hannes Reinecke wrote:
> [ .. ]
> >>
> >> We really would need some indicator whether 'ccr' is supported at all.
> > 
> > Why do we need this indicator, other than exporting it via sysfs?
> > 
> To avoid false positives.

We will never try CCR on a controller that does not support it. False
positive of what?

> 
> >> Using the number of available CCR commands would be an option, if though
> >> that would require us to keep two counters (one for the number of
> >> possible outstanding CCRs, and one for the number of actual outstanding
> >> CCRs.).
> > 
> > Like mentioned above ctrl->ccr_limit gives us the number of ccrs
> > available now. It is not 100% indicator if CCR is supported or not, but
> > it is enough to implement CCR. A second counter can help us skip trying
> > CCR if we know impacted controller does not support it.
> > 
> > Do you think it is worth it?
> > 
> Yes. The problem is that we want to get towards TP8028 compliance, which
> forces us to wait for 2*KATO + CQT before requests on the failed patch
> can be retried. That will cause a _noticeable_ stall on the application
> side. And the only way to shorten that is CCR; once we get confirmation
> from CCR we can start retrying immediately.
> At the same time the current implementation only waits for 1*KATO before
> retrying, so there will be regression if we switch to TP8028-compliant
> KATO handling for systems not supporting CCR.

The statement above is not correct. Careful consideration and testing
has been made to not introduce such regression. If CCR is not supported
then nvme_find_ctrl_ccr() will return NULL and nvme_fence_ctrl() will
return immediately. No CCR command will be sent and no wait for AEN.

What happens next depends on whether ictrl->cqt is supported or not. If
not supported, which will be the case for systems in the field today,
then requests will be retried immediately. Requests will not be held in
this case and no delay will be seen in failover case.

> 
> So we can (and should) use CCR as the determining factor whether we
> want to switch to TP8028-compliant behaviour or stick with the original
> implementation.

We do check CCR support and availability in nvme_find_ctrl_ccr(). Adding
a second counter will spare us the loop in nvme_find_ctrl_ccr(), which
is not worth it IMO.

> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke                  Kernel Storage Architect
> hare@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
Re: [PATCH v3 09/21] nvme: Implement cross-controller reset completion
Posted by Randy Jennings 1 month, 1 week ago 
On Wed, Feb 18, 2026 at 4:47 AM Mohamed Khalfella
<mkhalfella@purestorage.com> wrote:
>
> On Wed 2026-02-18 08:51:31 +0100, Hannes Reinecke wrote:
> > On 2/17/26 19:25, Mohamed Khalfella wrote:
> At the same time the current implementation only waits for 1*KATO before
> retrying, so there will be regression if we switch to TP8028-compliant
> KATO handling for systems not supporting CCR.
Hannes, as I read the code (this is patch 19), if CQT is not set,
there is no delay. I
was expecting that to continue forward (I would be happy to exclude
'1' also).  I agree that we would not want to use CQT where subsystems
have not requested that time to quiesce.

Am I reading this wrong, and you are worried that committed code currently waits
for 1*KATO, and this patch set shortens that?  I do not see a delay of 1*KATO in
committed code. What am I missing?

> > > On Mon 2026-02-16 13:43:51 +0100, Hannes Reinecke wrote:

> > So we can (and should) use CCR as the determining factor whether we
> > want to switch to TP8028-compliant behaviour or stick with the original
> > implementation.
>
> We do check CCR support and availability in nvme_find_ctrl_ccr(). Adding
> a second counter will spare us the loop in nvme_find_ctrl_ccr(), which
> is not worth it IMO.

Another option is the Commands Supported log page.  CCR is a command,
so support for it should show up there.  The data structure is not the
simplest to reference; it might end up more complicated than having a
separate flag (why use another counter?),

RE:
> > want to switch to TP8028-compliant behaviour or stick with the original
> > implementation.
Hannes, do you mean TP8028 or TP4129?  Yes, if we do not support CCRs
we should not send them or expect to receive a successful response.

I would be careful of stating this in terms of TP-compliant behavior.  I
care about fixing a data corruption.  TP4129 worked out what that
required and provided a channel to communicate how long the
subsystem took to clean up, but I really do not care much about
compliance outside of compatibility and predictability.  As long as the
data corruption is handled conclusively and in a feasible manner
(IOW, no, the subsystem cannot clean up instantaneously, and we
do have to deal with possible communication delays while
coordinating between the host and subsystem), I can be happy
with the solution.

Sincerely,
Randy Jennings

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.