Re: [PATCH v3 00/14] Pass data temperature information to SCSI disk devices

[Damien Le Moal <dlemoal@kernel.org>]

On 10/19/23 04:34, Bart Van Assche wrote:
> 
> On 10/18/23 12:09, Jens Axboe wrote:
>> My main hesitation with this is that there's a big gap between what
>> makes theoretical sense and practical sense. When we previously tried
>> this, turns out devices retained the data temperature on media, as
>> expected, but tossed it out when data was GC'ed. That made it more of a
>> benchmarking case than anything else. How do we know that things are
>> better now? In previous postings I've seen you point at some papers, but
>> I'm mostly concerned with practical use cases and devices. Are there any
>> results, at all, from that? Or is this a case of vendors asking for
>> something to check some marketing boxes or have value add?
> 
> Hi Jens,
> 
> Multiple UFS vendors made it clear to me that this feature is essential 
> for their UFS devices to perform well. I will reach out to some of these
> vendors off-list and will ask them to share performance numbers.
> 
> A note: persistent stream support is a feature that was only added
> recently in the latest SCSI SBC-5 draft. This SCSI specification change
> allows SCSI device vendors to interpret the GROUP NUMBER field as a data
> lifetime. UFS device vendors interpret the GROUP NUMBER field as a data
> lifetime since a long time - long before this was allowed by the SCSI
> standards. See also the "ContextID" feature in the UFS specification.
> That feature is mentioned in every version of the UFS specification I
> have access to. The oldest version of the UFS specification I have
> access to is version 2.2, published in 2016.
> (https://www.jedec.org/system/files/docs/JESD220C-2_2.pdf). This
> document is available free of charge after an account has been created 
> on the JEDEC website.
> 
>> I'm also really against growing struct bio just for this. Why is patch 2
>> not just using the ioprio field at least?
> 
> Hmm ... shouldn't the bits in the ioprio field in struct bio have the
> same meaning as in the ioprio fields used in interfaces between user
> space and the kernel? Damien Le Moal asked me not to use any of the
> ioprio bits passing data lifetime information from user space to the kernel.

I said so in the context that if lifetime is a per-inode property, then ioprio
is the wrong interface since the ioprio API is per process or per IO. There is a
mismatch.

One version of your patch series used fnctl() to set the lifetime per inoe,
which is fine, and then used the BIO ioprio to pass the lifetime down to the
device driver. That is in theory a nice trick, but that creates conflicts with
the userspace ioprio API if the user uses that at the same time.

So may be we should change bio ioprio from int to u16 and use the freedup u16
for lifetime. With that, things are cleanly separated without growing struct bio.

> 
> Is it clear that the size of struct bio has not been changed because the
> new bi_lifetime member fills a hole in struct bio?

When the struct is randomized, holes move or disappear. Don't count on that...

> 
> Thanks,
> 
> Bart.
> 
> 

-- 
Damien Le Moal
Western Digital Research