The fabric transports and also the PCI transport are not entering the
LIVE state from NEW or RESETTING. This makes the state machine more
restrictive and allows to catch not supported state transitions, e.g.
directly switching from RESETTING to LIVE.
Signed-off-by: Daniel Wagner <wagi@kernel.org>
---
drivers/nvme/host/core.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 818d4e49aab51c388af9a48bf9d466fea9cef51b..f028913e2e622ee348e88879c6e6b7e8f8a1cc82 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -564,8 +564,6 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
switch (new_state) {
case NVME_CTRL_LIVE:
switch (old_state) {
- case NVME_CTRL_NEW:
- case NVME_CTRL_RESETTING:
case NVME_CTRL_CONNECTING:
changed = true;
fallthrough;
--
2.48.1Unfortunately, it has been discovered that this patch causes a serious regression on powerpc platforms.
If anyone has a powerpc platform with an NVMe/PCIe device installed, please run this simple test and see if it works.
# uname -av
Linux rdma-cert-03-lp10.rdma.lab.eng.rdu2.redhat.com 6.19.0-rc4+ #1 SMP Wed Jan 7 21:42:54 EST 2026 ppc64le GNU/Linux
# nvme list-subsys /dev/nvme0n1
nvme-subsys0 - NQN=nqn.1994-11.com.samsung:nvme:PM1735:HHHL:S4WANA0R400032
hostnqn=nqn.2014-08.org.nvmexpress:uuid:1654a627-93b6-4650-ba90-f4dc7a2fd3ee
iopolicy=numa
\
+- nvme0 pcie 0018:01:00.0 live optimized
# nvme subsystem-reset /dev/nvme0; nvme list-subsys /dev/nvme0n1; sleep 1; nvme list-subsys /dev/nvme0n1; nvme list-subsys /dev/nvme0n1;
nvme-subsys0 - NQN=nqn.1994-11.com.samsung:nvme:PM1735:HHHL:S4WANA0R400032
hostnqn=nqn.2014-08.org.nvmexpress:uuid:1654a627-93b6-4650-ba90-f4dc7a2fd3ee
iopolicy=numa
\
+- nvme0 pcie 0018:01:00.0 resetting optimized
[Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
[Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
[Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
[Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
[Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
# nvme list-subsys /dev/nvme0n1;
# nvme list-subsys /dev/nvme0n1;
nvme-subsys0 - NQN=nqn.1994-11.com.samsung:nvme:PM1735:HHHL:S4WANA0R400032
hostnqn=nqn.2014-08.org.nvmexpress:uuid:1654a627-93b6-4650-ba90-f4dc7a2fd3ee
iopolicy=numa
\
+- nvme0 pcie 0018:01:00.0 resetting optimized
At this point the machine is HUNG. It's stuck in the resetting state forever.
Because /dev/nvme0n1 is the root device, I need to power-cycle/reboot the host to recover.
/John
On 2/14/25 3:02 AM, Daniel Wagner wrote:
> The fabric transports and also the PCI transport are not entering the
> LIVE state from NEW or RESETTING. This makes the state machine more
> restrictive and allows to catch not supported state transitions, e.g.
> directly switching from RESETTING to LIVE.
>
> Signed-off-by: Daniel Wagner <wagi@kernel.org>
> ---
> drivers/nvme/host/core.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 818d4e49aab51c388af9a48bf9d466fea9cef51b..f028913e2e622ee348e88879c6e6b7e8f8a1cc82 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -564,8 +564,6 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
> switch (new_state) {
> case NVME_CTRL_LIVE:
> switch (old_state) {
> - case NVME_CTRL_NEW:
> - case NVME_CTRL_RESETTING:
> case NVME_CTRL_CONNECTING:
> changed = true;
> fallthrough;
>
On 1/10/26 12:48 AM, John Meneghini wrote:
> Unfortunately, it has been discovered that this patch causes a serious regression on powerpc platforms.
>
> If anyone has a powerpc platform with an NVMe/PCIe device installed, please run this simple test and see if it works.
>
> # uname -av
> Linux rdma-cert-03-lp10.rdma.lab.eng.rdu2.redhat.com 6.19.0-rc4+ #1 SMP Wed Jan 7 21:42:54 EST 2026 ppc64le GNU/Linux
>
> # nvme list-subsys /dev/nvme0n1
> nvme-subsys0 - NQN=nqn.1994-11.com.samsung:nvme:PM1735:HHHL:S4WANA0R400032
> hostnqn=nqn.2014-08.org.nvmexpress:uuid:1654a627-93b6-4650-ba90-f4dc7a2fd3ee
> iopolicy=numa
> \
> +- nvme0 pcie 0018:01:00.0 live optimized
>
> # nvme subsystem-reset /dev/nvme0; nvme list-subsys /dev/nvme0n1; sleep 1; nvme list-subsys /dev/nvme0n1; nvme list-subsys /dev/nvme0n1;
> nvme-subsys0 - NQN=nqn.1994-11.com.samsung:nvme:PM1735:HHHL:S4WANA0R400032
> hostnqn=nqn.2014-08.org.nvmexpress:uuid:1654a627-93b6-4650-ba90-f4dc7a2fd3ee
> iopolicy=numa
> \
> +- nvme0 pcie 0018:01:00.0 resetting optimized
> [Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
> [Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
> [Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
> [Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
> [Wed Jan 7 21:59:51 2026] block nvme0n1: no usable path - requeuing I/O
>
> # nvme list-subsys /dev/nvme0n1;
>
> # nvme list-subsys /dev/nvme0n1;
> nvme-subsys0 - NQN=nqn.1994-11.com.samsung:nvme:PM1735:HHHL:S4WANA0R400032
> hostnqn=nqn.2014-08.org.nvmexpress:uuid:1654a627-93b6-4650-ba90-f4dc7a2fd3ee
> iopolicy=numa
> \
> +- nvme0 pcie 0018:01:00.0 resetting optimized
>
> At this point the machine is HUNG. It's stuck in the resetting state forever.
>
> Because /dev/nvme0n1 is the root device, I need to power-cycle/reboot the host to recover.
> /John
>
> On 2/14/25 3:02 AM, Daniel Wagner wrote:
>> The fabric transports and also the PCI transport are not entering the
>> LIVE state from NEW or RESETTING. This makes the state machine more
>> restrictive and allows to catch not supported state transitions, e.g.
>> directly switching from RESETTING to LIVE.
>>
>> Signed-off-by: Daniel Wagner <wagi@kernel.org>
>> ---
>> drivers/nvme/host/core.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 818d4e49aab51c388af9a48bf9d466fea9cef51b..f028913e2e622ee348e88879c6e6b7e8f8a1cc82 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -564,8 +564,6 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
>> switch (new_state) {
>> case NVME_CTRL_LIVE:
>> switch (old_state) {
>> - case NVME_CTRL_NEW:
>> - case NVME_CTRL_RESETTING:
>> case NVME_CTRL_CONNECTING:
>> changed = true;
>> fallthrough;
>>
>
This was broken because with this commit d2fe192348f9 (“nvme: only allow entering LIVE
from CONNECTING state”) now we don't allow changing controller state from
RESETTING -> LIVE. I saw we also had similar state change issue with firmware activation
code which was fixed by explicitly transitioning the controller state through RESETTING ->
CONNECTING -> LIVE. We may employ the similar solution here for subsystem reset case as well.
Currently, the NVMe PCIe subsystem reset code performs the following steps:
1. Sets the controller state to RESETTING
2. Writes the subsystem reset command to the NSSR register
3. Attempts to transition the controller state directly to LIVE
This effectively bypasses the CONNECTING state. The transition to LIVE is artificial but
intentional, since writing to the NSSR register causes the loss of communication with the
NVMe adapter and the controller must be marked LIVE so that any in-flight I/O at the time the
subsystem reset is issued, or an explicit MMIO read, can trigger EEH recovery and ultimately
restore communication link between the NVMe adapter and the system.
With the stricter state transition rules introduced by commit d2fe192348f9 (“nvme: only allow
entering LIVE from CONNECTING state”), the direct transition from RESETTING -> LIVE is no longer
permitted, rendering the current logic ineffective.
Taking a cue from the firmware activation fix, it seems reasonable to explicitly transition
the controller state through CONNECTING in the subsystem reset path as well. So how about making
the following change to fix this?
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 0e4caeab739c..3027bba232de 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -1532,7 +1532,10 @@ static int nvme_pci_subsystem_reset(struct nvme_ctrl *ctrl)
}
writel(NVME_SUBSYS_RESET, dev->bar + NVME_REG_NSSR);
- nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE);
+
+ if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
+ !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
+ goto unlock;
/*
* Read controller status to flush the previous write and trigger a
Thanks,
--Nilay
Hi Nilay,
On Sun, Jan 11, 2026 at 03:03:43PM +0530, Nilay Shroff wrote:
> This was broken because with this commit d2fe192348f9 (“nvme: only allow entering LIVE
> from CONNECTING state”) now we don't allow changing controller state from
> RESETTING -> LIVE. I saw we also had similar state change issue with firmware activation
> code which was fixed by explicitly transitioning the controller state through RESETTING ->
> CONNECTING -> LIVE. We may employ the similar solution here for subsystem reset case as well.
>
> Currently, the NVMe PCIe subsystem reset code performs the following steps:
>
> 1. Sets the controller state to RESETTING
> 2. Writes the subsystem reset command to the NSSR register
> 3. Attempts to transition the controller state directly to LIVE
>
> This effectively bypasses the CONNECTING state. The transition to LIVE is artificial but
> intentional, since writing to the NSSR register causes the loss of communication with the
> NVMe adapter and the controller must be marked LIVE so that any in-flight I/O at the time the
> subsystem reset is issued, or an explicit MMIO read, can trigger EEH recovery and ultimately
> restore communication link between the NVMe adapter and the system.
>
> With the stricter state transition rules introduced by commit d2fe192348f9 (“nvme: only allow
> entering LIVE from CONNECTING state”), the direct transition from RESETTING -> LIVE is no longer
> permitted, rendering the current logic ineffective.
>
> Taking a cue from the firmware activation fix, it seems reasonable to explicitly transition
> the controller state through CONNECTING in the subsystem reset path as well. So how about making
> the following change to fix this?
>
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 0e4caeab739c..3027bba232de 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1532,7 +1532,10 @@ static int nvme_pci_subsystem_reset(struct nvme_ctrl *ctrl)
> }
>
> writel(NVME_SUBSYS_RESET, dev->bar + NVME_REG_NSSR);
> - nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE);
> +
> + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
> + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
> + goto unlock;
>
> /*
> * Read controller status to flush the previous write and trigger a
This seems to be similar to case where the firmware update got stuck [1].
650415fca0a9 ("nvme: unblock ctrl state transition for firmware update")
From my understanding this should work, but it's probably better to
double check that this doesn't violate any assumptions.
[1] https://lore.kernel.org/all/aBJJQoOBhaXj7P36@kbusch-mbp/
Thanks,
Daniel
On 1/12/26 1:44 PM, Daniel Wagner wrote:
> Hi Nilay,
>
> On Sun, Jan 11, 2026 at 03:03:43PM +0530, Nilay Shroff wrote:
>> This was broken because with this commit d2fe192348f9 (“nvme: only allow entering LIVE
>> from CONNECTING state”) now we don't allow changing controller state from
>> RESETTING -> LIVE. I saw we also had similar state change issue with firmware activation
>> code which was fixed by explicitly transitioning the controller state through RESETTING ->
>> CONNECTING -> LIVE. We may employ the similar solution here for subsystem reset case as well.
>>
>> Currently, the NVMe PCIe subsystem reset code performs the following steps:
>>
>> 1. Sets the controller state to RESETTING
>> 2. Writes the subsystem reset command to the NSSR register
>> 3. Attempts to transition the controller state directly to LIVE
>>
>> This effectively bypasses the CONNECTING state. The transition to LIVE is artificial but
>> intentional, since writing to the NSSR register causes the loss of communication with the
>> NVMe adapter and the controller must be marked LIVE so that any in-flight I/O at the time the
>> subsystem reset is issued, or an explicit MMIO read, can trigger EEH recovery and ultimately
>> restore communication link between the NVMe adapter and the system.
>>
>> With the stricter state transition rules introduced by commit d2fe192348f9 (“nvme: only allow
>> entering LIVE from CONNECTING state”), the direct transition from RESETTING -> LIVE is no longer
>> permitted, rendering the current logic ineffective.
>>
>> Taking a cue from the firmware activation fix, it seems reasonable to explicitly transition
>> the controller state through CONNECTING in the subsystem reset path as well. So how about making
>> the following change to fix this?
>>
>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
>> index 0e4caeab739c..3027bba232de 100644
>> --- a/drivers/nvme/host/pci.c
>> +++ b/drivers/nvme/host/pci.c
>> @@ -1532,7 +1532,10 @@ static int nvme_pci_subsystem_reset(struct nvme_ctrl *ctrl)
>> }
>>
>> writel(NVME_SUBSYS_RESET, dev->bar + NVME_REG_NSSR);
>> - nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE);
>> +
>> + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
>> + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
>> + goto unlock;
>>
>> /*
>> * Read controller status to flush the previous write and trigger a
>
> This seems to be similar to case where the firmware update got stuck [1].
>
> 650415fca0a9 ("nvme: unblock ctrl state transition for firmware update")
>
> From my understanding this should work, but it's probably better to
> double check that this doesn't violate any assumptions.
>
> [1] https://lore.kernel.org/all/aBJJQoOBhaXj7P36@kbusch-mbp/
>
I have tested this (and I believe John as well) and this seems tobe working good. From my code walk through, the controller state
transition RESETTING -> CONNECTING -> LIVE looks good to me, so I'll
send out a formal patch for review.
Thanks,
--Nilay
On 1/13/26 1:10 AM, Nilay Shroff wrote: > I have tested this (and I believe John as well) and this seems tobe working good. From my code walk through, the controller state > transition RESETTING -> CONNECTING -> LIVE looks good to me, so I'll > send out a formal patch for review. Sounds good Nilay. Please send your patch for review and I will test it. /John
Hi,
On Fri, Feb 14, 2025 at 09:02:03AM +0100, Daniel Wagner wrote:
> The fabric transports and also the PCI transport are not entering the
> LIVE state from NEW or RESETTING. This makes the state machine more
> restrictive and allows to catch not supported state transitions, e.g.
> directly switching from RESETTING to LIVE.
>
> Signed-off-by: Daniel Wagner <wagi@kernel.org>
nvme_handle_aen_notice(), when handling NVME_AER_NOTICE_FW_ACT_STARTING,
sets the state to RESETTING and and triggers a worker. This worker
waits for firmware activation to complete and then tries to set the
state back to LIVE. This step now fails.
Possibly the handling of NVME_AER_NOTICE_FW_ACT_STARTING needs to be
improved. However, leaving the NVME in RESETTING state after an
NVME_AER_NOTICE_FW_ACT_STARTING event is worse.
I think this patch should be reverted at least for the time being until
the handling of NVME_AER_NOTICE_FW_ACT_STARTING no longer relies on a
direct state change from RESETTING to LIVE.
Please correct me if my analysis is wrong.
Thanks,
Guenter
> ---
> drivers/nvme/host/core.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 818d4e49aab51c388af9a48bf9d466fea9cef51b..f028913e2e622ee348e88879c6e6b7e8f8a1cc82 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -564,8 +564,6 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
> switch (new_state) {
> case NVME_CTRL_LIVE:
> switch (old_state) {
> - case NVME_CTRL_NEW:
> - case NVME_CTRL_RESETTING:
> case NVME_CTRL_CONNECTING:
> changed = true;
> fallthrough;
>
> --
> 2.48.1
>
On Sun, Apr 27, 2025 at 08:59:13AM -0700, Guenter Roeck wrote:
> Hi,
>
> On Fri, Feb 14, 2025 at 09:02:03AM +0100, Daniel Wagner wrote:
> > The fabric transports and also the PCI transport are not entering the
> > LIVE state from NEW or RESETTING. This makes the state machine more
> > restrictive and allows to catch not supported state transitions, e.g.
> > directly switching from RESETTING to LIVE.
> >
> > Signed-off-by: Daniel Wagner <wagi@kernel.org>
>
> nvme_handle_aen_notice(), when handling NVME_AER_NOTICE_FW_ACT_STARTING,
> sets the state to RESETTING and and triggers a worker. This worker
> waits for firmware activation to complete and then tries to set the
> state back to LIVE. This step now fails.
>
> Possibly the handling of NVME_AER_NOTICE_FW_ACT_STARTING needs to be
> improved. However, leaving the NVME in RESETTING state after an
> NVME_AER_NOTICE_FW_ACT_STARTING event is worse.
>
> I think this patch should be reverted at least for the time being until
> the handling of NVME_AER_NOTICE_FW_ACT_STARTING no longer relies on a
> direct state change from RESETTING to LIVE.
ee59e3820ca9 ("nvme-fc: do not ignore connectivity loss during connecting")
f13409bb3f91 ("nvme-fc: rely on state transitions to handle connectivity loss")
are depending on the fact that is not possible to switch from
NEW/RESETTING directly into LIVE.
I think it would be better to fix the worker instead dropping this patch
and the above fix for the fc transport.
What about:
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b502ac07483b..d3c4eacf607f 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work)
msleep(100);
}
- if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
+ if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
+ !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
return;
nvme_unquiesce_io_queues(ctrl);
On 4/28/25 14:44, Daniel Wagner wrote:
> On Sun, Apr 27, 2025 at 08:59:13AM -0700, Guenter Roeck wrote:
>> Hi,
>>
>> On Fri, Feb 14, 2025 at 09:02:03AM +0100, Daniel Wagner wrote:
>>> The fabric transports and also the PCI transport are not entering the
>>> LIVE state from NEW or RESETTING. This makes the state machine more
>>> restrictive and allows to catch not supported state transitions, e.g.
>>> directly switching from RESETTING to LIVE.
>>>
>>> Signed-off-by: Daniel Wagner <wagi@kernel.org>
>>
>> nvme_handle_aen_notice(), when handling NVME_AER_NOTICE_FW_ACT_STARTING,
>> sets the state to RESETTING and and triggers a worker. This worker
>> waits for firmware activation to complete and then tries to set the
>> state back to LIVE. This step now fails.
>>
>> Possibly the handling of NVME_AER_NOTICE_FW_ACT_STARTING needs to be
>> improved. However, leaving the NVME in RESETTING state after an
>> NVME_AER_NOTICE_FW_ACT_STARTING event is worse.
>>
>> I think this patch should be reverted at least for the time being until
>> the handling of NVME_AER_NOTICE_FW_ACT_STARTING no longer relies on a
>> direct state change from RESETTING to LIVE.
>
> ee59e3820ca9 ("nvme-fc: do not ignore connectivity loss during connecting")
> f13409bb3f91 ("nvme-fc: rely on state transitions to handle connectivity loss")
>
> are depending on the fact that is not possible to switch from
> NEW/RESETTING directly into LIVE.
>
> I think it would be better to fix the worker instead dropping this patch
> and the above fix for the fc transport.
>
> What about:
>
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index b502ac07483b..d3c4eacf607f 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work)
> msleep(100);
> }
>
> - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
> + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
> + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
> return;
>
> nvme_unquiesce_io_queues(ctrl);
I would rather have a separate state for firmware activation.
(Ab-)using the 'RESETTING' state here has direct implications
with the error handler, as for the error handler 'RESETTING'
means that the error handler has been scheduled.
Which is not true for firmware activation.
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
On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote: > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > > index b502ac07483b..d3c4eacf607f 100644 > > --- a/drivers/nvme/host/core.c > > +++ b/drivers/nvme/host/core.c > > @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work) > > msleep(100); > > } > > > > - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) > > + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) || > > + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) > > return; > > > > nvme_unquiesce_io_queues(ctrl); > > I would rather have a separate state for firmware activation. > (Ab-)using the 'RESETTING' state here has direct implications > with the error handler, as for the error handler 'RESETTING' > means that the error handler has been scheduled. > Which is not true for firmware activation. But the point of having firmware activation set the state to RESETTING was to fence off error handling from trying to schedule a real reset. The fw activation work schedules its own recovery if it times out, but we don't want any other recovery action or user requested resets to proceed while an activation is still pending.
On 4/29/25 20:13, Keith Busch wrote: > On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote: >>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c >>> index b502ac07483b..d3c4eacf607f 100644 >>> --- a/drivers/nvme/host/core.c >>> +++ b/drivers/nvme/host/core.c >>> @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work) >>> msleep(100); >>> } >>> >>> - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) >>> + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) || >>> + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) >>> return; >>> >>> nvme_unquiesce_io_queues(ctrl); >> >> I would rather have a separate state for firmware activation. >> (Ab-)using the 'RESETTING' state here has direct implications >> with the error handler, as for the error handler 'RESETTING' >> means that the error handler has been scheduled. >> Which is not true for firmware activation. > > But the point of having firmware activation set the state to RESETTING > was to fence off error handling from trying to schedule a real reset. > The fw activation work schedules its own recovery if it times out, but > we don't want any other recovery action or user requested resets to > proceed while an activation is still pending. I know; that was precisely my point. We are overloading the 'RESETTTING' state to mean either 'reset has started' or 'fw activation is ongoing'. Which are two _vastly_ different situations, and we should differentiate them eg by introducing a new state. That new state can (and should) have the same effects as the RESETTING state, true. 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
On 4/29/25 11:13, Keith Busch wrote: > On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote: >>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c >>> index b502ac07483b..d3c4eacf607f 100644 >>> --- a/drivers/nvme/host/core.c >>> +++ b/drivers/nvme/host/core.c >>> @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work) >>> msleep(100); >>> } >>> >>> - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) >>> + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) || >>> + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) >>> return; >>> >>> nvme_unquiesce_io_queues(ctrl); >> >> I would rather have a separate state for firmware activation. >> (Ab-)using the 'RESETTING' state here has direct implications >> with the error handler, as for the error handler 'RESETTING' >> means that the error handler has been scheduled. >> Which is not true for firmware activation. > > But the point of having firmware activation set the state to RESETTING > was to fence off error handling from trying to schedule a real reset. > The fw activation work schedules its own recovery if it times out, but > we don't want any other recovery action or user requested resets to > proceed while an activation is still pending. Not only that; there are various checks against NVME_CTRL_RESETTING sprinkled through the code. What is the impact of introducing a new state without handling all those checks ? Guenter
On Tue, Apr 29, 2025 at 11:23:25AM -0700, Guenter Roeck wrote: > On 4/29/25 11:13, Keith Busch wrote: > > On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote: > > > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > > > > index b502ac07483b..d3c4eacf607f 100644 > > > > --- a/drivers/nvme/host/core.c > > > > +++ b/drivers/nvme/host/core.c > > > > @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work) > > > > msleep(100); > > > > } > > > > > > > > - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) > > > > + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) || > > > > + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) > > > > return; > > > > > > > > nvme_unquiesce_io_queues(ctrl); > > > > > > I would rather have a separate state for firmware activation. > > > (Ab-)using the 'RESETTING' state here has direct implications > > > with the error handler, as for the error handler 'RESETTING' > > > means that the error handler has been scheduled. > > > Which is not true for firmware activation. > > > > But the point of having firmware activation set the state to RESETTING > > was to fence off error handling from trying to schedule a real reset. > > The fw activation work schedules its own recovery if it times out, but > > we don't want any other recovery action or user requested resets to > > proceed while an activation is still pending. > > Not only that; there are various checks against NVME_CTRL_RESETTING > sprinkled through the code. What is the impact of introducing a new state > without handling all those checks ? Good point, bad things will happen if these checks are not updated to know about the new state. For example, nvme-pci will attempt aborting IO or disabling the controller on a timeout instead of restarting the timer as desired. Can we just revert the commit that prevented the RESETTING -> LIVE transtion for now?
On Tue, Apr 29, 2025 at 11:42:25AM -0700, Keith Busch wrote:
> On Tue, Apr 29, 2025 at 11:23:25AM -0700, Guenter Roeck wrote:
> > On 4/29/25 11:13, Keith Busch wrote:
> > > On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote:
> > > > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > > > > index b502ac07483b..d3c4eacf607f 100644
> > > > > --- a/drivers/nvme/host/core.c
> > > > > +++ b/drivers/nvme/host/core.c
> > > > > @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work)
> > > > > msleep(100);
> > > > > }
> > > > >
> > > > > - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
> > > > > + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
> > > > > + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
> > > > > return;
> > > > >
> > > > > nvme_unquiesce_io_queues(ctrl);
> > > >
> > > > I would rather have a separate state for firmware activation.
> > > > (Ab-)using the 'RESETTING' state here has direct implications
> > > > with the error handler, as for the error handler 'RESETTING'
> > > > means that the error handler has been scheduled.
> > > > Which is not true for firmware activation.
> > >
> > > But the point of having firmware activation set the state to RESETTING
> > > was to fence off error handling from trying to schedule a real reset.
> > > The fw activation work schedules its own recovery if it times out, but
> > > we don't want any other recovery action or user requested resets to
> > > proceed while an activation is still pending.
> >
> > Not only that; there are various checks against NVME_CTRL_RESETTING
> > sprinkled through the code. What is the impact of introducing a new state
> > without handling all those checks ?
>
> Good point, bad things will happen if these checks are not updated to
> know about the new state. For example, nvme-pci will attempt aborting IO
> or disabling the controller on a timeout instead of restarting the timer
> as desired.
>
> Can we just revert the commit that prevented the RESETTING -> LIVE
> transtion for now?
Unfortunately, it will break the FC error handling again(*). The
simplest fix is right above, add the transition from RESETTING to
CONNECTING and then to LIVE, IMO.
(*) ee59e3820ca9 ("nvme-fc: do not ignore connectivity loss during connecting")
f13409bb3f91 ("nvme-fc: rely on state transitions to handle connectivity loss")
On 4/29/25 23:43, Daniel Wagner wrote:
> On Tue, Apr 29, 2025 at 11:42:25AM -0700, Keith Busch wrote:
>> On Tue, Apr 29, 2025 at 11:23:25AM -0700, Guenter Roeck wrote:
>>> On 4/29/25 11:13, Keith Busch wrote:
>>>> On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote:
>>>>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>>>>> index b502ac07483b..d3c4eacf607f 100644
>>>>>> --- a/drivers/nvme/host/core.c
>>>>>> +++ b/drivers/nvme/host/core.c
>>>>>> @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work)
>>>>>> msleep(100);
>>>>>> }
>>>>>>
>>>>>> - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
>>>>>> + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
>>>>>> + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
>>>>>> return;
>>>>>>
>>>>>> nvme_unquiesce_io_queues(ctrl);
>>>>>
>>>>> I would rather have a separate state for firmware activation.
>>>>> (Ab-)using the 'RESETTING' state here has direct implications
>>>>> with the error handler, as for the error handler 'RESETTING'
>>>>> means that the error handler has been scheduled.
>>>>> Which is not true for firmware activation.
>>>>
>>>> But the point of having firmware activation set the state to RESETTING
>>>> was to fence off error handling from trying to schedule a real reset.
>>>> The fw activation work schedules its own recovery if it times out, but
>>>> we don't want any other recovery action or user requested resets to
>>>> proceed while an activation is still pending.
>>>
>>> Not only that; there are various checks against NVME_CTRL_RESETTING
>>> sprinkled through the code. What is the impact of introducing a new state
>>> without handling all those checks ?
>>
>> Good point, bad things will happen if these checks are not updated to
>> know about the new state. For example, nvme-pci will attempt aborting IO
>> or disabling the controller on a timeout instead of restarting the timer
>> as desired.
>>
>> Can we just revert the commit that prevented the RESETTING -> LIVE
>> transtion for now?
>
> Unfortunately, it will break the FC error handling again(*). The
> simplest fix is right above, add the transition from RESETTING to
> CONNECTING and then to LIVE, IMO.
>
> (*) ee59e3820ca9 ("nvme-fc: do not ignore connectivity loss during connecting")
> f13409bb3f91 ("nvme-fc: rely on state transitions to handle connectivity loss")
I don't know if having nvme-fc _depend_ on state transition restrictions is good
or bad, and I don't know anything about the NVME state machine.
However, given that there _is_ a real regression in the code right now, I agree
that the solution (or call it workaround) suggested by Daniel (or something
similar) should be the way to go, and it would be easy to backport. After all,
firmware update is now broken in all stable release branches, and that is not
a good situation to be in.
Introducing a new state for firmware updates should be done carefully and not be
part of a regression fix which needs to be backported to all stable release
branches.
Thanks,
Guenter
On Wed, Apr 30, 2025 at 08:43:04AM +0200, Daniel Wagner wrote: > On Tue, Apr 29, 2025 at 11:42:25AM -0700, Keith Busch wrote: > > On Tue, Apr 29, 2025 at 11:23:25AM -0700, Guenter Roeck wrote: > > > On 4/29/25 11:13, Keith Busch wrote: > > > > On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote: > > > > > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > > > > > > index b502ac07483b..d3c4eacf607f 100644 > > > > > > --- a/drivers/nvme/host/core.c > > > > > > +++ b/drivers/nvme/host/core.c > > > > > > @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work) > > > > > > msleep(100); > > > > > > } > > > > > > > > > > > > - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) > > > > > > + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) || > > > > > > + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) > > > > > > return; > > > > > > > > > > > > nvme_unquiesce_io_queues(ctrl); > > > > > > > > > > I would rather have a separate state for firmware activation. > > > > > (Ab-)using the 'RESETTING' state here has direct implications > > > > > with the error handler, as for the error handler 'RESETTING' > > > > > means that the error handler has been scheduled. > > > > > Which is not true for firmware activation. > > > > > > > > But the point of having firmware activation set the state to RESETTING > > > > was to fence off error handling from trying to schedule a real reset. > > > > The fw activation work schedules its own recovery if it times out, but > > > > we don't want any other recovery action or user requested resets to > > > > proceed while an activation is still pending. > > > > > > Not only that; there are various checks against NVME_CTRL_RESETTING > > > sprinkled through the code. What is the impact of introducing a new state > > > without handling all those checks ? > > > > Good point, bad things will happen if these checks are not updated to > > know about the new state. For example, nvme-pci will attempt aborting IO > > or disabling the controller on a timeout instead of restarting the timer > > as desired. > > > > Can we just revert the commit that prevented the RESETTING -> LIVE > > transtion for now? > > Unfortunately, it will break the FC error handling again(*). The > simplest fix is right above, add the transition from RESETTING to > CONNECTING and then to LIVE, IMO. Gotcha, yes, that looks like the simplest fix for the current release then. We need to be careful with adding new states, so we can revisit Hannes' suggestion for 6.16 if we really want to split this. If you send the simple fix as a formal patch, please add my review and the "Fixes:" tag. Reviewed-by: Keith Busch <kbusch@kernel.org>
On 4/30/25 09:01, Keith Busch wrote: > On Wed, Apr 30, 2025 at 08:43:04AM +0200, Daniel Wagner wrote: >> On Tue, Apr 29, 2025 at 11:42:25AM -0700, Keith Busch wrote: >>> On Tue, Apr 29, 2025 at 11:23:25AM -0700, Guenter Roeck wrote: >>>> On 4/29/25 11:13, Keith Busch wrote: >>>>> On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote: >>>>>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c >>>>>>> index b502ac07483b..d3c4eacf607f 100644 >>>>>>> --- a/drivers/nvme/host/core.c >>>>>>> +++ b/drivers/nvme/host/core.c >>>>>>> @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work) >>>>>>> msleep(100); >>>>>>> } >>>>>>> >>>>>>> - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) >>>>>>> + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) || >>>>>>> + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) >>>>>>> return; >>>>>>> >>>>>>> nvme_unquiesce_io_queues(ctrl); >>>>>> >>>>>> I would rather have a separate state for firmware activation. >>>>>> (Ab-)using the 'RESETTING' state here has direct implications >>>>>> with the error handler, as for the error handler 'RESETTING' >>>>>> means that the error handler has been scheduled. >>>>>> Which is not true for firmware activation. >>>>> >>>>> But the point of having firmware activation set the state to RESETTING >>>>> was to fence off error handling from trying to schedule a real reset. >>>>> The fw activation work schedules its own recovery if it times out, but >>>>> we don't want any other recovery action or user requested resets to >>>>> proceed while an activation is still pending. >>>> >>>> Not only that; there are various checks against NVME_CTRL_RESETTING >>>> sprinkled through the code. What is the impact of introducing a new state >>>> without handling all those checks ? >>> >>> Good point, bad things will happen if these checks are not updated to >>> know about the new state. For example, nvme-pci will attempt aborting IO >>> or disabling the controller on a timeout instead of restarting the timer >>> as desired. >>> >>> Can we just revert the commit that prevented the RESETTING -> LIVE >>> transtion for now? >> >> Unfortunately, it will break the FC error handling again(*). The >> simplest fix is right above, add the transition from RESETTING to >> CONNECTING and then to LIVE, IMO. > > Gotcha, yes, that looks like the simplest fix for the current release > then. We need to be careful with adding new states, so we can revisit > Hannes' suggestion for 6.16 if we really want to split this. > > If you send the simple fix as a formal patch, please add my review and > the "Fixes:" tag. > > Reviewed-by: Keith Busch <kbusch@kernel.org> Please feel free to add mine as well: Reviewed-by: Guenter Roeck <linux@roeck-us.net>
On Wed, Apr 30, 2025 at 09:12:30AM -0700, Guenter Roeck wrote: > On 4/30/25 09:01, Keith Busch wrote: > > If you send the simple fix as a formal patch, please add my review and > > the "Fixes:" tag. > > > > Reviewed-by: Keith Busch <kbusch@kernel.org> > > Please feel free to add mine as well: > > Reviewed-by: Guenter Roeck <linux@roeck-us.net> Sorry for the delay, yesterday was a public holiday and way to nice weather to do any work. Just sent out the fix. Thanks for the review tags.
On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote:
> On 4/28/25 14:44, Daniel Wagner wrote:
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index b502ac07483b..d3c4eacf607f 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work)
> > msleep(100);
> > }
> >
> > - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
> > + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
> > + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
> > return;
> >
> > nvme_unquiesce_io_queues(ctrl);
>
> I would rather have a separate state for firmware activation.
> (Ab-)using the 'RESETTING' state here has direct implications
> with the error handler, as for the error handler 'RESETTING'
> means that the error handler has been scheduled.
> Which is not true for firmware activation.
Okay, so something like this here (untested, working on it)?
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b502ac07483b..32482712d0f2 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -565,6 +565,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
case NVME_CTRL_LIVE:
switch (old_state) {
case NVME_CTRL_CONNECTING:
+ case NVME_CTRL_FW_ACTIVATION:
changed = true;
fallthrough;
default:
@@ -575,6 +576,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
switch (old_state) {
case NVME_CTRL_NEW:
case NVME_CTRL_LIVE:
+ case NVME_CTRL_FW_ACTIVATION:
changed = true;
fallthrough;
default:
@@ -596,6 +598,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
case NVME_CTRL_LIVE:
case NVME_CTRL_RESETTING:
case NVME_CTRL_CONNECTING:
+ case NVME_CTRL_FW_ACTIVATION:
changed = true;
fallthrough;
default:
@@ -621,6 +624,15 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
break;
}
break;
+ case NVME_CTRL_FW_ACTIVATION:
+ switch (old_state) {
+ case NVME_CTRL_LIVE:
+ changed = true;
+ fallthrough;
+ default:
+ break;
+ }
+ break;
default:
break;
}
@@ -4529,7 +4541,7 @@ static bool nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
* recovery actions from interfering with the controller's
* firmware activation.
*/
- if (nvme_change_ctrl_state(ctrl, NVME_CTRL_RESETTING)) {
+ if (nvme_change_ctrl_state(ctrl, NVME_CTRL_FW_ACTIVATION)) {
requeue = false;
queue_work(nvme_wq, &ctrl->fw_act_work);
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 51e078642127..3a383225afed 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -247,6 +247,7 @@ static inline u16 nvme_req_qid(struct request *req)
* shutdown or removal. In this case we forcibly
* kill all inflight I/O as they have no chance to
* complete
+ * @NVME_CTRL_FW_ACTIVATION: Controller is in firmware activation state.
*/
enum nvme_ctrl_state {
NVME_CTRL_NEW,
@@ -256,6 +257,7 @@ enum nvme_ctrl_state {
NVME_CTRL_DELETING,
NVME_CTRL_DELETING_NOIO,
NVME_CTRL_DEAD,
+ NVME_CTRL_FW_ACTIVATION,
};
struct nvme_fault_inject {
On 4/29/25 15:55, Daniel Wagner wrote:
> On Mon, Apr 28, 2025 at 03:21:18PM +0200, Hannes Reinecke wrote:
>> On 4/28/25 14:44, Daniel Wagner wrote:
>>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>>> index b502ac07483b..d3c4eacf607f 100644
>>> --- a/drivers/nvme/host/core.c
>>> +++ b/drivers/nvme/host/core.c
>>> @@ -4493,7 +4493,8 @@ static void nvme_fw_act_work(struct work_struct *work)
>>> msleep(100);
>>> }
>>>
>>> - if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
>>> + if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING) ||
>>> + !nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE))
>>> return;
>>>
>>> nvme_unquiesce_io_queues(ctrl);
>>
>> I would rather have a separate state for firmware activation.
>> (Ab-)using the 'RESETTING' state here has direct implications
>> with the error handler, as for the error handler 'RESETTING'
>> means that the error handler has been scheduled.
>> Which is not true for firmware activation.
>
> Okay, so something like this here (untested, working on it)?
>
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index b502ac07483b..32482712d0f2 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -565,6 +565,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
> case NVME_CTRL_LIVE:
> switch (old_state) {
> case NVME_CTRL_CONNECTING:
> + case NVME_CTRL_FW_ACTIVATION:
> changed = true;
> fallthrough;
> default:
> @@ -575,6 +576,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
> switch (old_state) {
> case NVME_CTRL_NEW:
> case NVME_CTRL_LIVE:
> + case NVME_CTRL_FW_ACTIVATION:
> changed = true;
> fallthrough;
> default:
> @@ -596,6 +598,7 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
> case NVME_CTRL_LIVE:
> case NVME_CTRL_RESETTING:
> case NVME_CTRL_CONNECTING:
> + case NVME_CTRL_FW_ACTIVATION:
> changed = true;
> fallthrough;
> default:
> @@ -621,6 +624,15 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
> break;
> }
> break;
> + case NVME_CTRL_FW_ACTIVATION:
> + switch (old_state) {
> + case NVME_CTRL_LIVE:
> + changed = true;
> + fallthrough;
> + default:
> + break;
> + }
> + break;
> default:
> break;
> }
> @@ -4529,7 +4541,7 @@ static bool nvme_handle_aen_notice(struct nvme_ctrl *ctrl, u32 result)
> * recovery actions from interfering with the controller's
> * firmware activation.
> */
> - if (nvme_change_ctrl_state(ctrl, NVME_CTRL_RESETTING)) {
> + if (nvme_change_ctrl_state(ctrl, NVME_CTRL_FW_ACTIVATION)) {
> requeue = false;
> queue_work(nvme_wq, &ctrl->fw_act_work);
> }
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 51e078642127..3a383225afed 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -247,6 +247,7 @@ static inline u16 nvme_req_qid(struct request *req)
> * shutdown or removal. In this case we forcibly
> * kill all inflight I/O as they have no chance to
> * complete
> + * @NVME_CTRL_FW_ACTIVATION: Controller is in firmware activation state.
> */
> enum nvme_ctrl_state {
> NVME_CTRL_NEW,
> @@ -256,6 +257,7 @@ enum nvme_ctrl_state {
> NVME_CTRL_DELETING,
> NVME_CTRL_DELETING_NOIO,
> NVME_CTRL_DEAD,
> + NVME_CTRL_FW_ACTIVATION,
> };
>
> struct nvme_fault_inject {
>
Indeed, something like it.
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
© 2016 - 2026 Red Hat, Inc.