Re: [PATCH v8 7/8] nvme: sysfs: emit the marginal path state in show_state()

[Keith Busch <kbusch@kernel.org>]

On Tue, Jul 15, 2025 at 03:42:32PM -0400, John Meneghini wrote:
> On 7/9/25 6:12 PM, Keith Busch wrote:
> > On Wed, Jul 09, 2025 at 05:19:18PM -0400, Bryan Gurney wrote:
> > > If a controller has received a link integrity or congestion event, and
> > > has the NVME_CTRL_MARGINAL flag set, emit "marginal" in the state
> > > instead of "live", to identify the marginal paths.
> > 
> > IMO, this attribute looks more aligned to report in the ana_state
> > instead of overriding the controller's state.
> > 
> 
> We can't really do this because the ANA state is a documented protocol state.
> 
> The linux controller state is purely a linux software defined state.  Unless I am wrong, there is nothing in the NVMe specification which defines the nvme_ctrl_state.

Totally correct.
 
> This is purely a linux definition and we should be able to change is any way we want.

My kneejerk reaction is against adding new controller states. We have
state checks sprinkled about, and special states just make that more
fragile.
 
> We debated adding a new NVME_CTRL_MARGINAL state to this data structure,
> 
> enum nvme_ctrl_state {
>         NVME_CTRL_NEW,
>         NVME_CTRL_LIVE,
>         NVME_CTRL_RESETTING,
>         NVME_CTRL_CONNECTING,
>         NVME_CTRL_DELETING,
>         NVME_CTRL_DELETING_NOIO,
>         NVME_CTRL_DEAD,
> };
> 
> If you don't like the flag we can do that. However, that doesn't seem worth the effort since Hannes has this working now with a flag.

What you're describing is a "path" state, not a controller state which
is why I'm suggesting the "ana_state" attribute since nothing else
represents the path fitness. If nvme can't describe this condition, then
maybe it should?

Where does this 'FPIN LI' message originate from? The end point or
something inbetween? If it's the endpoint (or if both sides get the same
message?), then an ANA state to non-optimal should be possible, no? And
we already have the infrastructure to react to changing ANA states, so
you can transition to optimal if something gets repaired.