Re: [PATCH 1/2] nvme: make independent ns identify default

[Matias Bjørling]

On 10-10-2024 16:47, Daniel Wagner wrote:
> On Wed, Oct 09, 2024 at 03:19:59PM GMT, Matias Bjørling wrote:
> > On 09-10-2024 09:46, Christoph Hellwig wrote:
> > > On Tue, Oct 08, 2024 at 04:55:02PM +0200, Matias Bjørling wrote:
> > > > However, the independent namespace data structure
> > > > is mandatory for devices that implement features from the 2.0+
> > > > specification. Therefore, we can check this data structure first. If
> > > > unavailable, retrieve the generic attributes from the NVM command set
> > > > identify namespace data structure.
> > >
> > > I'm not a huge fan of this.  For pre-2.0 controllers this means
> > > we'll now send a command that will fail most of them time.  And for
> > > all the cheap low-end consumer device I'm actually worried that they'll
> > > get it wrong and break something.
> >
> > It's a good point. Damien, Keith, and I were debating it during ALPSS. They
> > preferred the "send command and see if it fails" approach over writing
> > specific conditions where it would apply. Note that Keith did suggest to
> > avoid the command on 1.0 and 1.1 devices, and they were known to fail with
> > unsupported CNS ids.
>
> FWIW, there are some devices out there which will log these attempts and
> spam their error logs. There were plenty of reports against nvme-cli
> when nvme-cli issued a command which could fail.

Got it. So, given the feedback from you, Keith, and Christoph. It's safe 
to say it needs to be a conditional check.

Would anyone object if the

 if ((ctrl->cap & NVME_CAP_CRMS_CRIMS) ||
     (info.ids.csi != NVME_CSI_NVM && info.ids.csi != NVME_CSI_ZNS))

statement would include a check for endurance group support?

The idea being that it's mandatory for a device to implement an 
endurance group in case it exposes the rotational flag. That check would 
limit the failed command check to relatively new devices.