| Message ID | 20231114214132.1486867-2-bvanassche@acm.org |
|---|---|
| State | New |
| Headers | show |
| Series | Pass data lifetime information to SCSI disk devices | expand |
On Tue, Nov 14, 2023 at 01:40:56PM -0800, Bart Van Assche wrote: > - case WRITE_LIFE_SHORT: > + case WRITE_LIFE_2: > return CURSEG_HOT_DATA; > - case WRITE_LIFE_EXTREME: > + case WRITE_LIFE_5: > return CURSEG_COLD_DATA; > default: > return CURSEG_WARM_DATA; A WRITE_LIFE_2 constant is strictly more confusing than just using 2, so this patch makes no sense whatsoever. More importantly these constant have been around forever, so we'd better have a really good argument for changing them.
On 11/27/2023 12:38 PM, Christoph Hellwig wrote: > On Tue, Nov 14, 2023 at 01:40:56PM -0800, Bart Van Assche wrote: >> - case WRITE_LIFE_SHORT: >> + case WRITE_LIFE_2: >> return CURSEG_HOT_DATA; >> - case WRITE_LIFE_EXTREME: >> + case WRITE_LIFE_5: >> return CURSEG_COLD_DATA; >> default: >> return CURSEG_WARM_DATA; > > A WRITE_LIFE_2 constant is strictly more confusing than just using 2, > so this patch makes no sense whatsoever. > > More importantly these constant have been around forever, so we'd better > have a really good argument for changing them. How about this argument (assuming you may not have seen) from previous iteration [1]- "Does it make sense to do away with these, and have temperature-neutral names instead e.g., WRITE_LIFE_1, WRITE_LIFE_2? With the current choice: - If the count goes up (beyond 5 hints), infra can scale fine but these names do not. Imagine ULTRA_EXTREME after EXTREME. - Applications or in-kernel users can specify LONG hint with data that actually has a SHORT lifetime. Nothing really ensures that LONG is really LONG. Temperature-neutral names seem more generic/scalable and do not present the unnecessary need to be accurate with relative temperatures." [1] https://lore.kernel.org/linux-block/b3058ce6-e297-b4c3-71d4-4b76f76439ba@samsung.com/
On 11/26/23 23:08, Christoph Hellwig wrote: > More importantly these constant have been around forever, so we'd better > have a really good argument for changing them. Hi Christoph, I will drop this patch. As you know the NVMe and SCSI specifications use the numeric range 0..63 for the data lifetime so there is a gap between the values supported by the F_[GS]ET_RW_HINT fcntls and the data lifetime values accepted by widely used storage devices. Do you think that it should be possible for user space applications to specify the full range (0..63)? Thanks, Bart.
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 727d016318f9..098a574d8d84 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -3281,9 +3281,9 @@ int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range) int f2fs_rw_hint_to_seg_type(enum rw_hint hint) { switch (hint) { - case WRITE_LIFE_SHORT: + case WRITE_LIFE_2: return CURSEG_HOT_DATA; - case WRITE_LIFE_EXTREME: + case WRITE_LIFE_5: return CURSEG_COLD_DATA; default: return CURSEG_WARM_DATA; diff --git a/fs/inode.c b/fs/inode.c index edcd8a61975f..7965d5e07012 100644 --- a/fs/inode.c +++ b/fs/inode.c @@ -175,7 +175,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode) i_gid_write(inode, 0); atomic_set(&inode->i_writecount, 0); inode->i_size = 0; - inode->i_write_hint = WRITE_LIFE_NOT_SET; + inode->i_write_hint = WRITE_LIFE_0; inode->i_blocks = 0; inode->i_bytes = 0; inode->i_generation = 0; diff --git a/include/linux/fs.h b/include/linux/fs.h index 98b7a7a8c42e..59f9de9df0fe 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -314,12 +314,12 @@ struct readahead_control; * Stored in struct inode as u8. */ enum rw_hint { - WRITE_LIFE_NOT_SET = 0, - WRITE_LIFE_NONE = RWH_WRITE_LIFE_NONE, - WRITE_LIFE_SHORT = RWH_WRITE_LIFE_SHORT, - WRITE_LIFE_MEDIUM = RWH_WRITE_LIFE_MEDIUM, - WRITE_LIFE_LONG = RWH_WRITE_LIFE_LONG, - WRITE_LIFE_EXTREME = RWH_WRITE_LIFE_EXTREME, + WRITE_LIFE_0 = 0, + WRITE_LIFE_1 = RWH_WRITE_LIFE_NONE, + WRITE_LIFE_2 = RWH_WRITE_LIFE_SHORT, + WRITE_LIFE_3 = RWH_WRITE_LIFE_MEDIUM, + WRITE_LIFE_4 = RWH_WRITE_LIFE_LONG, + WRITE_LIFE_5 = RWH_WRITE_LIFE_EXTREME, }; /* Match RWF_* bits to IOCB bits */
Prepare for supporting more data lifetimes by changing data lifetime names into numeric constants. Cc: Kanchan Joshi <joshi.k@samsung.com> Cc: Jan Kara <jack@suse.cz> Cc: Christoph Hellwig <hch@lst.de> Cc: Christian Brauner <brauner@kernel.org> Suggested-by: Kanchan Joshi <joshi.k@samsung.com> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- fs/f2fs/segment.c | 4 ++-- fs/inode.c | 2 +- include/linux/fs.h | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-)