| Message ID | 20230920191442.3701673-1-bvanassche@acm.org |
|---|---|
| Headers | show |
| Series | Pass data temperature information to zoned UFS devices | expand |
On 09/21, Matthew Wilcox wrote: > On Thu, Sep 21, 2023 at 12:39:00PM -0700, Bart Van Assche wrote: > > On 9/21/23 12:27, Matthew Wilcox wrote: > > > On Thu, Sep 21, 2023 at 07:27:08AM -0700, Bart Van Assche wrote: > > > > On 9/21/23 00:46, Niklas Cassel wrote: > > > > > Should NVMe streams be brought back? Yes? No? > > > > > > > > From commit 561593a048d7 ("Merge tag 'for-5.18/write-streams-2022-03-18' > > > > of git://git.kernel.dk/linux-block"): "This removes the write streams > > > > support in NVMe. No vendor ever really shipped working support for this, > > > > and they are not interested in supporting it." > > > > > > It sounds like UFS is at the same stage that NVMe got to -- standard > > > exists, no vendor has committed to actually shipping it. Isn't bringing > > > it back a little premature? > > > > Hi Matthew, > > > > That's a misunderstanding. UFS vendors support interpreting the SCSI GROUP > > NUMBER as a data temperature since many years, probably since more than ten > > years. Additionally, for multiple UFS vendors having the data temperature > > available is important for achieving good performance. This message shows > > how UFS vendors were using that information before write hint support was > > removed: https://lore.kernel.org/linux-block/PH0PR08MB7889642784B2E1FC1799A828DB0B9@PH0PR08MB7889.namprd08.prod.outlook.com/ > > If vendor support already exists, then why did you dodge the question > asking for quantified data that I asked earlier? And can we have that > data now? I'm in doubt this patch-set really requires the quantified data which may be mostly confidential to all the companies, also given the revert reason was no user, IIUC. OTOH, I'm not sure whether you're famailiar with FTL, but, when we consider the entire stack ranging from f2fs to FTL which manages NAND blocks, I do see a clear benefit to give the temperature hints for FTL to align therein garbage collection unit with one in f2fs, which is the key idea on Zoned UFS in mobile world, I believe. Otherwise, it can show non-deterministic longer write latencies due to internal GCs, increase WAI feeding to shorter lifetime.
Hi Bart! > Zoned UFS vendors need the data temperature information. Hence this > patch series that restores write hint information in F2FS and in the > block layer. The SCSI disk (sd) driver is modified such that it passes > write hint information to SCSI devices via the GROUP NUMBER field. I don't have any particular problems with your implementation, although I'm still trying to wrap my head around how to make this coexist with my I/O hinting series. But I guess there's probably not going to be a big overlap between devices that support both features. However, it still pains me greatly to see the SBC proposal being intertwined with the travesty that is streams. Why not define everything in the IO advice hints group descriptor? I/O hints already use GROUP NUMBER as an index. Why not just define a few permanent hint descriptors? What's the point of the additional level of indirection to tie this new feature into streams? RSCS basically says "ignore the streams-specific bits and bobs and do this other stuff instead". What does the streams infrastructure provide that can't be solved trivially in the IO advise mode page alone? For existing UFS devices which predate RSCS and streams but which support getting data temperature from GROUP NUMBER, what is the mechanism for detecting and enabling the feature?
On 9/27/23 12:14, Martin K. Petersen wrote: > I don't have any particular problems with your implementation, > although I'm still trying to wrap my head around how to make this > coexist with my I/O hinting series. But I guess there's probably not > going to be a big overlap between devices that support both > features. Hi Martin, This patch series should make it easier to implement I/O hint support since some of the code added by this patch series is also needed to implement I/O hint support. > However, it still pains me greatly to see the SBC proposal being > intertwined with the travesty that is streams. Why not define > everything in the IO advice hints group descriptor? I/O hints already > use GROUP NUMBER as an index. Why not just define a few permanent > hint descriptors? What's the point of the additional level of > indirection to tie this new feature into streams? RSCS basically says > "ignore the streams-specific bits and bobs and do this other stuff > instead". What does the streams infrastructure provide that can't be > solved trivially in the IO advise mode page alone? Hmm ... isn't that exactly what T10 did, define everything in the IO advice hints group descriptor by introducing the new ST_ENBLE bit in that descriptor? This patch series relies on the constrained streams mechanism. A constrained stream is permanently open. The new ST_ENBLE bit in the IO advice hints group descriptor indicates whether or not an IO advice hints group represents a permanent stream. The new ST_ENBLE bit in the IO advice hints group descriptor allows SCSI devices to interpret the index of the descriptor as a data lifetime. From the approved T10 proposal: Table x1 – RELATIVE LIFETIME field .............................................. Code Relative lifetime .............................................. 00h no relative lifetime is applicable 01h shortest relative lifetime 02h second shortest relative lifetime 03h to 3Dh intermediate relative lifetimes 3Eh second longest relative lifetime 3Fh longest relative lifetime .............................................. > For existing UFS devices which predate RSCS and streams but which > support getting data temperature from GROUP NUMBER, what is the > mechanism for detecting and enabling the feature? We plan to ask UFS device vendors to modify the UFS device firmware and to add support for the VPD and mode pages this patch series relies on. My understanding is that this can be done easily in UFS device firmware. Although it is technically possible to update the firmware of UFS devices in smartphones, most smartphones do not support this because this is considered risky. Hence, only new smartphones will benefit from this patch series. I do not want to add support in the Linux kernel for how conventional UFS devices use the GROUP NUMBER field today. Conventional UFS devices interpret the GROUP NUMBER field as a "ContextID". The ContextID mechanism has a state, just like the SCSI stream mechanism. UFS contexts need to be opened explicitly and are closed upon reset. From the UFS 4.0 specification: "No ContextID shall be open after power cycle." Please let me know if you need more information. Bart.
On Wed, Sep 27, 2023 at 03:14:10PM -0400, Martin K. Petersen wrote: > > Hi Bart! > > > Zoned UFS vendors need the data temperature information. Hence this > > patch series that restores write hint information in F2FS and in the > > block layer. The SCSI disk (sd) driver is modified such that it passes > > write hint information to SCSI devices via the GROUP NUMBER field. > > I don't have any particular problems with your implementation, although > I'm still trying to wrap my head around how to make this coexist with my > I/O hinting series. But I guess there's probably not going to be a big > overlap between devices that support both features. Hello Bart, Martin, I don't know which user facing API Martin's I/O hinting series is intending to use. However, while discussing this series at ALPSS, we did ask ourselves why this series is not reusing the already existing block layer API for providing I/O hints: https://github.com/torvalds/linux/blob/v6.6-rc4/include/uapi/linux/ioprio.h#L83-L103 We can have 1023 possible I/O hints, and so far we are only using 7, which means that there are 1016 possible hints left. This also enables you to define more than the 4 previous temperature hints (extreme, long, medium, short), if so desired. There is also support in fio for these I/O hints: https://github.com/axboe/fio/blob/master/HOWTO.rst?plain=1#L2294-L2302 When this new I/O hint API has added, there was no other I/O hint API in the kernel (since the old fcntl() F_GET_FILE_RW_HINT / F_SET_FILE_RW_HINT API had already been removed when this new API was added). So there should probably be a good argument why we would want to introduce yet another API for providing I/O hints, instead of extending the I/O hint API that we already have in the kernel right now. (Especially since it seems fairly easy to modify your patches to reuse the existing API.) Kind regards, Niklas
On Mon, Oct 02, 2023 at 01:37:59PM +0200, Niklas Cassel wrote: > On Wed, Sep 27, 2023 at 03:14:10PM -0400, Martin K. Petersen wrote: > > > > Hi Bart! > > > > > Zoned UFS vendors need the data temperature information. Hence this > > > patch series that restores write hint information in F2FS and in the > > > block layer. The SCSI disk (sd) driver is modified such that it passes > > > write hint information to SCSI devices via the GROUP NUMBER field. > > > > I don't have any particular problems with your implementation, although > > I'm still trying to wrap my head around how to make this coexist with my > > I/O hinting series. But I guess there's probably not going to be a big > > overlap between devices that support both features. > > Hello Bart, Martin, > > I don't know which user facing API Martin's I/O hinting series is intending > to use. > > However, while discussing this series at ALPSS, we did ask ourselves why this > series is not reusing the already existing block layer API for providing I/O > hints: > https://github.com/torvalds/linux/blob/v6.6-rc4/include/uapi/linux/ioprio.h#L83-L103 > > We can have 1023 possible I/O hints, and so far we are only using 7, which > means that there are 1016 possible hints left. > This also enables you to define more than the 4 previous temperature hints > (extreme, long, medium, short), if so desired. > > There is also support in fio for these I/O hints: > https://github.com/axboe/fio/blob/master/HOWTO.rst?plain=1#L2294-L2302 > > When this new I/O hint API has added, there was no other I/O hint API > in the kernel (since the old fcntl() F_GET_FILE_RW_HINT / F_SET_FILE_RW_HINT > API had already been removed when this new API was added). > > So there should probably be a good argument why we would want to introduce > yet another API for providing I/O hints, instead of extending the I/O hint > API that we already have in the kernel right now. > (Especially since it seems fairly easy to modify your patches to reuse the > existing API.) One argument might be that the current I/O hints API does not allow hints to be stacked. So one would not e.g. be able to combine a command duration limit with a temperature hint... Kind regards, Niklas
On 10/2/23 04:53, Niklas Cassel wrote: > On Mon, Oct 02, 2023 at 01:37:59PM +0200, Niklas Cassel wrote: >> I don't know which user facing API Martin's I/O hinting series is intending >> to use. >> >> However, while discussing this series at ALPSS, we did ask ourselves why this >> series is not reusing the already existing block layer API for providing I/O >> hints: >> https://github.com/torvalds/linux/blob/v6.6-rc4/include/uapi/linux/ioprio.h#L83-L103 >> >> We can have 1023 possible I/O hints, and so far we are only using 7, which >> means that there are 1016 possible hints left. >> This also enables you to define more than the 4 previous temperature hints >> (extreme, long, medium, short), if so desired. >> >> There is also support in fio for these I/O hints: >> https://github.com/axboe/fio/blob/master/HOWTO.rst?plain=1#L2294-L2302 >> >> When this new I/O hint API has added, there was no other I/O hint API >> in the kernel (since the old fcntl() F_GET_FILE_RW_HINT / F_SET_FILE_RW_HINT >> API had already been removed when this new API was added). >> >> So there should probably be a good argument why we would want to introduce >> yet another API for providing I/O hints, instead of extending the I/O hint >> API that we already have in the kernel right now. >> (Especially since it seems fairly easy to modify your patches to reuse the >> existing API.) > > One argument might be that the current I/O hints API does not allow hints to > be stacked. So one would not e.g. be able to combine a command duration limit > with a temperature hint... Hi Niklas, Is your feedback about the user space API only or also about the mechanism that is used internally in the kernel? Restoring the ability to pass data temperature information from a filesystem to a block device is much more important to me than restoring the ability to pass data temperature information from user space to a filesystem. Would it be sufficient to address your concern if patch 2/13 would be dropped from this series? Thanks, Bart.
On 10/2/23 04:38, Niklas Cassel wrote: > So there should probably be a good argument why we would want to > introduce yet another API for providing I/O hints, instead of > extending the I/O hint API that we already have in the kernel right > now. (Especially since it seems fairly easy to modify your patches > to reuse the existing API.) Here is a strong argument: there is user space software that is using the F_SET_FILE_RW_HINT API, e.g. Samba. I don't think that the above arguments are strong enough to tell all developers of user space software to switch from F_SET_FILE_RW_HINT to another API. This would force user space developers to check the kernel version before they can decide which user space API to use. If the new user space API would get backported to distro kernels then that would cause a real nightmare for user space developers who want to use F_SET_FILE_RW_HINT or its equivalent. Thanks, Bart.
On Mon, Oct 02, 2023 at 09:33:22AM -0700, Bart Van Assche wrote: > On 10/2/23 04:53, Niklas Cassel wrote: > > On Mon, Oct 02, 2023 at 01:37:59PM +0200, Niklas Cassel wrote: > > > I don't know which user facing API Martin's I/O hinting series is intending > > > to use. > > > > > > However, while discussing this series at ALPSS, we did ask ourselves why this > > > series is not reusing the already existing block layer API for providing I/O > > > hints: > > > https://github.com/torvalds/linux/blob/v6.6-rc4/include/uapi/linux/ioprio.h#L83-L103 > > > > > > We can have 1023 possible I/O hints, and so far we are only using 7, which > > > means that there are 1016 possible hints left. > > > This also enables you to define more than the 4 previous temperature hints > > > (extreme, long, medium, short), if so desired. > > > > > > There is also support in fio for these I/O hints: > > > https://github.com/axboe/fio/blob/master/HOWTO.rst?plain=1#L2294-L2302 > > > > > > When this new I/O hint API has added, there was no other I/O hint API > > > in the kernel (since the old fcntl() F_GET_FILE_RW_HINT / F_SET_FILE_RW_HINT > > > API had already been removed when this new API was added). > > > > > > So there should probably be a good argument why we would want to introduce > > > yet another API for providing I/O hints, instead of extending the I/O hint > > > API that we already have in the kernel right now. > > > (Especially since it seems fairly easy to modify your patches to reuse the > > > existing API.) > > > > One argument might be that the current I/O hints API does not allow hints to > > be stacked. So one would not e.g. be able to combine a command duration limit > > with a temperature hint... > > Hi Niklas, > > Is your feedback about the user space API only or also about the > mechanism that is used internally in the kernel? The concern is only related to the user space API. (However, if you do reuse the existing I/O prio hints, you will avoid adding a new struct member to a lot of structs.) > > Restoring the ability to pass data temperature information from a > filesystem to a block device is much more important to me than > restoring the ability to pass data temperature information from user > space to a filesystem. Would it be sufficient to address your concern > if patch 2/13 would be dropped from this series? Right now 0 means no I/O hint. Value 1-7 is used for CDL. This means that bits 0-2 are currently used by CDL. I guess we could define e.g. bits 3-5 to be used by temperature hints, i.e. temperature hints could have values 0-7, where 0 would be no temperature hint. (I guess we could still limit the temperature hints to 1-4 if we want to keep the previous extreme/long/medium/short constants.) This way, we can combine a CDL value with a temperature hint. I.e. if user space has set bits in both bits 0-2 and 3-5, then both CDL and temperature hints are used. (And we would still have 4 bits left in 10 bit long I/O hints field that can be used by some other I/O hint feature in the future.) We could theoretically do this without changing the existing I/O prio hints API, as all the existing hints (CDL descriptors 1-7) would keep their existing values. While I think this sounds quite nice, since it would avoid what your patches currently do: adding a new "write_hint" struct member to the following structs: struct kiocb, struct file, struct request, struct request, struct bio. Instead it would rely on the existing ioprio struct members in these structs. Additionally you would not need to add code that avoid merging of requests with different write hints, as the current code already avoids merging of requests with different ioprio (which thus extends to ioprio I/O hints). Anyway, even if I do think that modifying your patch series to use the I/O prio hints API would be a simpler and cleaner solution, including a smaller diffstat, I do not care too strongly about this, and will leave the pondering to the very wise maintainers. Kind regards, Niklas
Niklas, > I don't know which user facing API Martin's I/O hinting series is > intending to use. I'm just using ioprio.
On 10/2/23 18:40, Martin K. Petersen wrote: > > Niklas, > >> I don't know which user facing API Martin's I/O hinting series is >> intending to use. > > I'm just using ioprio. Hi Martin, Do you plan to use existing bits from the ioprio bitmask or new bits? Bits 0-2 are used for the priority level. Bits 3-5 are used for CDL. Bits 13-15 are used for the I/O priority. The SCSI and NVMe standard define 64 different data lifetimes (six bits). So there are 16 - 3 - 3 - 6 = 4 remaining bits. Thanks, Bart.
On Tue, Oct 03, 2023 at 10:26:27AM -0700, Bart Van Assche wrote: > On 10/2/23 18:40, Martin K. Petersen wrote: > > > > Niklas, > > > > > I don't know which user facing API Martin's I/O hinting series is > > > intending to use. > > > > I'm just using ioprio. > > Hi Martin, > > Do you plan to use existing bits from the ioprio bitmask or new bits? Bits > 0-2 are used for the priority level. Bits 3-5 are used for CDL. Bits 13-15 > are used for the I/O priority. The SCSI and NVMe standard define 64 > different data lifetimes (six bits). So there are 16 - 3 - 3 - 6 = 4 > remaining bits. Hello Bart, I think the math is: 16 - 3 (prio level) - 3 (CDL) - 3 (prio class) = 7 so if we want 64 different values for data lifetimes (we previously only had 4 different values), that is 6 bits: 16 - 3 (prio level) - 3 (CDL) - 3 (prio class) - 6 (lifetime) = 1 so only one bit left for Martin :) Not very much room to play with... Kind regards, Niklas
Bart, > Do you plan to use existing bits from the ioprio bitmask or new bits? > Bits 0-2 are used for the priority level. Bits 3-5 are used for CDL. > Bits 13-15 are used for the I/O priority. The SCSI and NVMe standard > define 64 different data lifetimes (six bits). So there are 16 - 3 - 3 > - 6 = 4 remaining bits. I just use the existing I/O priority classes and levels to set a high/normal/low relative priority. I would still like pursue I/O classification since that performed better in our testing. But that does involve working with vendors on a Linux profile as discussed at LSF/MM. Don't really more than a handful in either case.