On 2023/01/26 6:23, Niklas Cassel wrote: > On Wed, Jan 25, 2023 at 10:37:52AM -0800, Bart Van Assche wrote: > > (snip) > >> Hi Damien, >> >> The more I think about this, the more I'm convinced that it would be wrong >> to introduce IOPRIO_CLASS_DL. Datacenters will have a mix of drives that >> support CDL and drives that do not support CDL. It seems wrong to me to >> make user space software responsible for figuring out whether or not the >> drive supports CDL before it can be decided which I/O priority class should >> be used. This is something the kernel should do instead of user space >> software. > > Well, if we take e.g. NCQ priority as an example, as that is probably > the only device side I/O priority feature currently supported by the > kernel. > > If you want to use of NCQ priority, you need to first enable > /sys/block/sdX/device/ncq_prio_enable > and then submit I/O using IOPRIO_CLASS_RT, so I would argue the user > already needs to know that a device supports device side I/O priority, > if he wants to make use of it. Yes, absolutely. In addition to this, NCQ high priority feature is optional. The host-level RT class scheduling works the same way regardless of a SATA drive supporting NCQ high priority or not. If ncq_prio_enable is not enabled (or not supported), the scheduler still works as before. If ncq_prio_enable is set for a drive that supports NCQ high prio, then the user gets the additional benefit of *also* having the drive prioritize the commands from high-priority user IOs. > For CDL there are 7 different limits for reads and 7 different > limits for writes, these limits can be configured by the user. > So the users that want to get most performance out of their drive > will most likely analyze their workloads, and set the limits depending > on how their workload actually looks like. > > Bottom line is that heavy users of CDL will absolutely know how the CDL > limits are configured in user space, as they will pick the correct CDL > index (prio level) for the descriptor that they want to use for the > specific I/O that they are doing. An ioscheduler will most likely be > disabled. Yes. And for cases where we still need an IO scheduler (e.g. SMR with mq-deadline), we really cannot use the priority level (CDL index) as a meaningful information to make request scheduling decisions because I think it is simply impossible to reliably define a "priority" order for the 7 read and write descriptors. We cannot map a set of 14 descriptors with a very large possible number of variations to sorted array of priority-like levels. > (For CDL, the limit is from the time the command is submitted to the device, > so from the device's PoV, it does not really matter if a command is queued > for a long time in a scheduler or not, but from an application PoV, it does > not make sense to hold back a command for long if it e.g. has a short limit.) > > > If we were to reuse IOPRIO_CLASS_RT, then I guess the best option would be > to have something like: > > $ cat /sys/block/sdX/device/rt_prio_backend > [none] ncq-prio cdl No need for this. We can keep the existing ncq_prio_enable and the proposed duration_limits/enable sysfs attributes. The user cannot enable both at the same time with our patches. So if the user enables ncq_prio_enable, then it will get high priority NCQ commands mapping for any level of the RT class. If duration_limits/enable is set, then the user will get CDL scheduling of commands on the drive. But again, the difficulty with this overloading is that we *cannot* implement a solid level-based scheduling in IO schedulers because ordering the CDLs in a meaningful way is impossible. So BFQ handling of the RT class would likely not result in the most ideal scheduling (that would depend heavily on how the CDL descriptors are defined on the drive). Hence my reluctance to overload the RT class for CDL. > Devices that does not support ncq-prio or cdl, > e.g. currently NVMe, would just have none > (i.e. RT simply means higher host side priority (if a scheduler is used)). Yes. Exactly. > SCSI would then have none and cdl > (for SCSI devices supporting CDL.) > > ATA would have none, ncq-prio and cdl. > (for ATA devices supporting CDL.) > > That would theoretically avoid another ioprio class, but like I've just > explained, a user space application making use of CDL would for sure know > how the descriptors look like anyway, so I'm not sure if there is an actual > benefit of doing it this way over simply having a IOPRIO_CLASS_DL. Agree. And as explained above, I think that reusing the RT class creates more problems than the only apparent simplification it is. > I guess the only benefit would be that we would avoid introducing another > I/O priority class (at the expense of additional complexity elsewhere). Yes. And I think that the added complexity to correctly handle the overloaded RT class is too much. RT class has been around for a long time for host-level IO priority scheduling. Let's not break it in weird ways. We certainly can work on improving handling of IOPRIO_CLASS_DL in IO schedulers. But in my opinion, that can be done later, after this initial series introducing CDL support is applied. -- Damien Le Moal Western Digital Research
