From: Jaegeuk Kim via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
To: Chao Yu <chao@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] f2fs: fix to parse temperature correctly in f2fs_get_segment_temp()
Date: Wed, 16 Oct 2024 16:07:24 +0000 [thread overview]
Message-ID: <Zw_kvFpU53626D90@google.com> (raw)
In-Reply-To: <20241016082038.607358-1-chao@kernel.org>
On 10/16, Chao Yu wrote:
> In __get_segment_type(), __get_segment_type_6() may return
> CURSEG_COLD_DATA_PINNED or CURSEG_ALL_DATA_ATGC log type, but
> following f2fs_get_segment_temp() can only handle persistent
> log type, fix it.
>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> fs/f2fs/f2fs.h | 5 +++--
> fs/f2fs/file.c | 4 ++--
> fs/f2fs/segment.c | 33 +++++++++++++++++++++++++--------
> fs/f2fs/segment.h | 4 ----
> 4 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index ce00cb546f4a..bda61d7ca8dd 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1019,7 +1019,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>
> -enum {
> +enum log_type {
> CURSEG_HOT_DATA = 0, /* directory entry blocks */
> CURSEG_WARM_DATA, /* data blocks */
> CURSEG_COLD_DATA, /* multimedia or GCed data blocks */
> @@ -3758,7 +3758,8 @@ void f2fs_replace_block(struct f2fs_sb_info *sbi, struct dnode_of_data *dn,
> block_t old_addr, block_t new_addr,
> unsigned char version, bool recover_curseg,
> bool recover_newaddr);
> -int f2fs_get_segment_temp(int seg_type);
> +enum temp_type f2fs_get_segment_temp(struct f2fs_sb_info *sbi,
> + enum log_type seg_type);
> int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
> block_t old_blkaddr, block_t *new_blkaddr,
> struct f2fs_summary *sum, int type,
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 0e07231dc093..92d7c62eba29 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -4858,8 +4858,8 @@ static void f2fs_dio_write_submit_io(const struct iomap_iter *iter,
> {
> struct inode *inode = iter->inode;
> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> - int seg_type = f2fs_rw_hint_to_seg_type(sbi, inode->i_write_hint);
> - enum temp_type temp = f2fs_get_segment_temp(seg_type);
> + enum log_type type = f2fs_rw_hint_to_seg_type(sbi, inode->i_write_hint);
> + enum temp_type temp = f2fs_get_segment_temp(sbi, type);
>
> bio->bi_write_hint = f2fs_io_type_to_rw_hint(sbi, DATA, temp);
> submit_bio(bio);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 859d70bbc5e7..3ed689157891 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -3582,18 +3582,35 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
> }
> }
>
> -int f2fs_get_segment_temp(int seg_type)
> +enum temp_type f2fs_get_segment_temp(struct f2fs_sb_info *sbi,
> + enum log_type type)
> {
> - if (IS_HOT(seg_type))
> - return HOT;
> - else if (IS_WARM(seg_type))
> - return WARM;
> - return COLD;
> + struct curseg_info *curseg = CURSEG_I(sbi, type);
> + enum temp_type temp;
To avoid static compilier complaint:
error: variable 'temp' is used uninitialized whenever switch default is taken
let's assign one.
enum temp_type temp = COLD;
> +
> + switch (curseg->seg_type) {
> + case CURSEG_HOT_NODE:
> + case CURSEG_HOT_DATA:
> + temp = HOT;
> + break;
> + case CURSEG_WARM_NODE:
> + case CURSEG_WARM_DATA:
> + temp = WARM;
> + break;
> + case CURSEG_COLD_NODE:
> + case CURSEG_COLD_DATA:
> + temp = COLD;
> + break;
> + default:
> + f2fs_bug_on(sbi, 1);
> + }
> +
> + return temp;
> }
>
> static int __get_segment_type(struct f2fs_io_info *fio)
> {
> - int type = 0;
> + enum log_type type;
>
> switch (F2FS_OPTION(fio->sbi).active_logs) {
> case 2:
> @@ -3609,7 +3626,7 @@ static int __get_segment_type(struct f2fs_io_info *fio)
> f2fs_bug_on(fio->sbi, true);
> }
>
> - fio->temp = f2fs_get_segment_temp(type);
> + fio->temp = f2fs_get_segment_temp(fio->sbi, type);
>
> return type;
> }
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 55a01da6c4be..6a23bb1d16a2 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -34,10 +34,6 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> f2fs_bug_on(sbi, seg_type >= NR_PERSISTENT_LOG);
> }
>
> -#define IS_HOT(t) ((t) == CURSEG_HOT_NODE || (t) == CURSEG_HOT_DATA)
> -#define IS_WARM(t) ((t) == CURSEG_WARM_NODE || (t) == CURSEG_WARM_DATA)
> -#define IS_COLD(t) ((t) == CURSEG_COLD_NODE || (t) == CURSEG_COLD_DATA)
> -
> #define IS_CURSEG(sbi, seg) \
> (((seg) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno) || \
> ((seg) == CURSEG_I(sbi, CURSEG_WARM_DATA)->segno) || \
> --
> 2.40.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] f2fs: fix to parse temperature correctly in f2fs_get_segment_temp()
Date: Wed, 16 Oct 2024 16:07:24 +0000 [thread overview]
Message-ID: <Zw_kvFpU53626D90@google.com> (raw)
In-Reply-To: <20241016082038.607358-1-chao@kernel.org>
On 10/16, Chao Yu wrote:
> In __get_segment_type(), __get_segment_type_6() may return
> CURSEG_COLD_DATA_PINNED or CURSEG_ALL_DATA_ATGC log type, but
> following f2fs_get_segment_temp() can only handle persistent
> log type, fix it.
>
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
> fs/f2fs/f2fs.h | 5 +++--
> fs/f2fs/file.c | 4 ++--
> fs/f2fs/segment.c | 33 +++++++++++++++++++++++++--------
> fs/f2fs/segment.h | 4 ----
> 4 files changed, 30 insertions(+), 16 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index ce00cb546f4a..bda61d7ca8dd 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1019,7 +1019,7 @@ static inline void set_new_dnode(struct dnode_of_data *dn, struct inode *inode,
> #define NR_CURSEG_PERSIST_TYPE (NR_CURSEG_DATA_TYPE + NR_CURSEG_NODE_TYPE)
> #define NR_CURSEG_TYPE (NR_CURSEG_INMEM_TYPE + NR_CURSEG_PERSIST_TYPE)
>
> -enum {
> +enum log_type {
> CURSEG_HOT_DATA = 0, /* directory entry blocks */
> CURSEG_WARM_DATA, /* data blocks */
> CURSEG_COLD_DATA, /* multimedia or GCed data blocks */
> @@ -3758,7 +3758,8 @@ void f2fs_replace_block(struct f2fs_sb_info *sbi, struct dnode_of_data *dn,
> block_t old_addr, block_t new_addr,
> unsigned char version, bool recover_curseg,
> bool recover_newaddr);
> -int f2fs_get_segment_temp(int seg_type);
> +enum temp_type f2fs_get_segment_temp(struct f2fs_sb_info *sbi,
> + enum log_type seg_type);
> int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
> block_t old_blkaddr, block_t *new_blkaddr,
> struct f2fs_summary *sum, int type,
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 0e07231dc093..92d7c62eba29 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -4858,8 +4858,8 @@ static void f2fs_dio_write_submit_io(const struct iomap_iter *iter,
> {
> struct inode *inode = iter->inode;
> struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> - int seg_type = f2fs_rw_hint_to_seg_type(sbi, inode->i_write_hint);
> - enum temp_type temp = f2fs_get_segment_temp(seg_type);
> + enum log_type type = f2fs_rw_hint_to_seg_type(sbi, inode->i_write_hint);
> + enum temp_type temp = f2fs_get_segment_temp(sbi, type);
>
> bio->bi_write_hint = f2fs_io_type_to_rw_hint(sbi, DATA, temp);
> submit_bio(bio);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 859d70bbc5e7..3ed689157891 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -3582,18 +3582,35 @@ static int __get_segment_type_6(struct f2fs_io_info *fio)
> }
> }
>
> -int f2fs_get_segment_temp(int seg_type)
> +enum temp_type f2fs_get_segment_temp(struct f2fs_sb_info *sbi,
> + enum log_type type)
> {
> - if (IS_HOT(seg_type))
> - return HOT;
> - else if (IS_WARM(seg_type))
> - return WARM;
> - return COLD;
> + struct curseg_info *curseg = CURSEG_I(sbi, type);
> + enum temp_type temp;
To avoid static compilier complaint:
error: variable 'temp' is used uninitialized whenever switch default is taken
let's assign one.
enum temp_type temp = COLD;
> +
> + switch (curseg->seg_type) {
> + case CURSEG_HOT_NODE:
> + case CURSEG_HOT_DATA:
> + temp = HOT;
> + break;
> + case CURSEG_WARM_NODE:
> + case CURSEG_WARM_DATA:
> + temp = WARM;
> + break;
> + case CURSEG_COLD_NODE:
> + case CURSEG_COLD_DATA:
> + temp = COLD;
> + break;
> + default:
> + f2fs_bug_on(sbi, 1);
> + }
> +
> + return temp;
> }
>
> static int __get_segment_type(struct f2fs_io_info *fio)
> {
> - int type = 0;
> + enum log_type type;
>
> switch (F2FS_OPTION(fio->sbi).active_logs) {
> case 2:
> @@ -3609,7 +3626,7 @@ static int __get_segment_type(struct f2fs_io_info *fio)
> f2fs_bug_on(fio->sbi, true);
> }
>
> - fio->temp = f2fs_get_segment_temp(type);
> + fio->temp = f2fs_get_segment_temp(fio->sbi, type);
>
> return type;
> }
> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
> index 55a01da6c4be..6a23bb1d16a2 100644
> --- a/fs/f2fs/segment.h
> +++ b/fs/f2fs/segment.h
> @@ -34,10 +34,6 @@ static inline void sanity_check_seg_type(struct f2fs_sb_info *sbi,
> f2fs_bug_on(sbi, seg_type >= NR_PERSISTENT_LOG);
> }
>
> -#define IS_HOT(t) ((t) == CURSEG_HOT_NODE || (t) == CURSEG_HOT_DATA)
> -#define IS_WARM(t) ((t) == CURSEG_WARM_NODE || (t) == CURSEG_WARM_DATA)
> -#define IS_COLD(t) ((t) == CURSEG_COLD_NODE || (t) == CURSEG_COLD_DATA)
> -
> #define IS_CURSEG(sbi, seg) \
> (((seg) == CURSEG_I(sbi, CURSEG_HOT_DATA)->segno) || \
> ((seg) == CURSEG_I(sbi, CURSEG_WARM_DATA)->segno) || \
> --
> 2.40.1
next prev parent reply other threads:[~2024-10-16 16:07 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-16 8:20 [f2fs-dev] [PATCH] f2fs: fix to parse temperature correctly in f2fs_get_segment_temp() Chao Yu via Linux-f2fs-devel
2024-10-16 8:20 ` Chao Yu
2024-10-16 16:07 ` Jaegeuk Kim via Linux-f2fs-devel [this message]
2024-10-16 16:07 ` Jaegeuk Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Zw_kvFpU53626D90@google.com \
--to=linux-f2fs-devel@lists.sourceforge.net \
--cc=chao@kernel.org \
--cc=jaegeuk@kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.