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

["Matias Bjørling" <m@bjorling.me>]

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.