Re: [PATCH v3 01/18] block: introduce duration-limits priority class

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/25/23 09:05, Bart Van Assche wrote:
> On 1/24/23 14:59, Damien Le Moal wrote:
>> There is only one priority class that ATA understands: RT (the level is
>> irrelevant and ignored). All RT class IOs are mapped to high priority NCQ
>> commands. All other classes map to normal priority (no priority bit set)
>> commands.
>>
>> And sure, we could map the level of RT class IOs to a CDL index, as we do
>> for the CDL class, but what would be the point ? The user should use the
>> CDL class in that case.
>>
>> Furthermore, there is one additional thing that we do not yet support but
>> will later: CDL descriptor 0 can be used to set a target time limit for
>> high priority NCQ commands. Without this new feature introduced with CDL,
>> the drive is free to schedule high priority NCQ commands as it wants, and
>> that is hard coded in FW. So you can endup with very aggressive scheduling
>> leading to significant overall IOPS drop and long tail latency for low
>> priority commands. See page 11 and 20 of this presentation for an example:
>>
>> https://www.snia.org/sites/default/files/SDC/2021/pdfs/SNIA-SDC21-LeMoal-Be-On-Time-command-duration-limits-Feature-Support-in%20Linux.pdf
>>
>> For a drive that supports both CDL and NCQ priority, with CDL feature
>> turned off, CDL descriptor 0 defines the time limit hint for high priority
>> NCQ commands. Again, CDL and NCQ high priority are mutually exclusive.
>>
>> So for clarity, I really would prefer separating CDL and RT classes as we
>> did. We could integrate CDL support reusing the RT class + level for CDL
>> index, but I think this may be very confusing for users, especially
>> considering that the CLDs on a drive can be defined in any order the user
>> wants, resulting in indexes/levels that does do not have any particular
>> order, making it impossible for the host to correctly schedule commands.
> 
> Hi Damien,
> 
> Thanks again for the detailed reply. Your replies are very informative 
> and help me understand the context better.
> 
> However, I'm still less than enthusiast about the introduction of the 
> I/O priority class IOPRIO_CLASS_DL. To me command duration limits (CDL) 
> is a mechanism that is supported by one storage standard (SCSI) and of 

And ATA (ACS) too. Not just SCSI. This is actually an improvement over IO
priority (command priority) that is supported only by ATA NCQ and does not
exist with SCSI/SBC.

> which it is not sure that it will be integrated in other storage 
> standards (NVMe, ...). Isn't the purpose of the block layer to provide 
> an interface that is independent of the specifics of a single storage 
> standard? This is why I'm in favor of letting the ATA core translate one 
> of the existing I/O priority classes into a CDL instead of introducing a 
> new I/O priority class (IOPRIO_CLASS_DL) in the block layer.

We discussed CDL with Hannes in the context of NVMe over fabrics. Their
may be interesting extensions to consider for NVMe in that context (the
value for local PCI attached NVMe drive is more limited at best).

I would argue that IO priority is the same: that is not supported by all
device classes either, and for those that support it, the semantic is not
identical (ATA vs NVMe). Yet, we have the RT class that maps to high
priority for ATA, and nothing else as far as I know.

CDL at least covers SCSI *and* ATA, and as mentioned above, could be used
by NVMe-of host drivers to do fancy link selection for a multipath setup
based on the link speed for instance.

We could overload the RT class with a mapping to CDL feature on scsi and
ata, but I think this is more confusing/messy than a separate class as we
implemented.

> 
> Others may have a different opinion.
> 
> Thanks,
> 
> Bart.

-- 
Damien Le Moal
Western Digital Research




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux