On Tue, Oct 29, 2024 at 04:53:30PM +0100, Christoph Hellwig wrote: > On Tue, Oct 29, 2024 at 09:38:44AM -0600, Keith Busch wrote: > > They're not exposed as write streams. Patch 7/9 sets the feature if it > > is a placement id or not, and only nvme sets it, so scsi's attributes > > are not claiming to be a write stream. > > So it shows up in sysfs, but: > > - queue_max_write_hints (which really should be queue_max_write_streams) > still picks it up, and from there the statx interface > > - per-inode fcntl hint that encode a temperature still magically > get dumpted into the write streams if they are set. > > In other words it's a really leaky half-backed abstraction. Exactly why I asked last time: "who uses it and how do you want them to use it" :) > Let's brainstorm how it could be done better: > > - the max_write_streams values only set by block devices that actually > do support write streams, and not the fire and forget temperature > hints. They way this is queried is by having a non-zero value > there, not need for an extra flag. So we need a completely different attribute for SCSI's permanent write streams? You'd mentioned earlier you were okay with having SCSI be able to utilized per-io raw block write hints. Having multiple things to check for what are all just write classifiers seems unnecessarily complicated. > - but the struct file (or maybe inode) gets a supported flag, as stream > separation needs to be supported by the file system > - a separate fcntl is used to set per-inode streams (if you care about > that, seem like the bdev use case focusses on per-I/O). In that case > we'd probably also need a separate inode field for them, or a somewhat > complicated scheme to decide what is stored in the inode field if there > is only one. No need to create a new fcntl. The people already testing this are successfully using FDP with the existing fcntl hints. Their applications leverage FDP as way to separate files based on expected lifetime. It is how they want to use it and it is working above expectations. > - for block devices bdev/fops.c maps the temperature hints into write > streams if write streams are supported, any user that mixes and > matches write streams and temperature hints gets what they deserve That's fine. This patch series pretty much accomplishes that part. > - this could also be a helper for file systems that want to do the > same. > > Just a quick writeup while I'm on the run, there's probably a hole or > two that could be poked into it.
