From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <yuchao0@huawei.com>
Cc: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: split f2fs_allocate_new_segments()
Date: Mon, 29 Jun 2020 13:19:43 -0700 [thread overview]
Message-ID: <20200629201943.GB1117827@google.com> (raw)
In-Reply-To: <20200622093849.35684-1-yuchao0@huawei.com>
On 06/22, Chao Yu wrote:
> to two independent functions:
> - f2fs_allocate_new_segment() for specified type segment allocation
> - f2fs_allocate_new_segments() for all data type segments allocation
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/f2fs.h | 3 ++-
> fs/f2fs/file.c | 2 +-
> fs/f2fs/recovery.c | 2 +-
> fs/f2fs/segment.c | 39 +++++++++++++++++++++++----------------
> 4 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 70565d81320b..07290943e91d 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3327,7 +3327,8 @@ void f2fs_release_discard_addrs(struct f2fs_sb_info *sbi);
> int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra);
> void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> unsigned int start, unsigned int end);
> -void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi, int type);
> +void f2fs_allocate_new_segment(struct f2fs_sb_info *sbi, int type);
> +void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi);
> int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range);
> bool f2fs_exist_trim_candidates(struct f2fs_sb_info *sbi,
> struct cp_control *cpc);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index f196187159e9..67c65e40b22b 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1659,7 +1659,7 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
> map.m_seg_type = CURSEG_COLD_DATA_PINNED;
>
> f2fs_lock_op(sbi);
> - f2fs_allocate_new_segments(sbi, CURSEG_COLD_DATA);
> + f2fs_allocate_new_segment(sbi, CURSEG_COLD_DATA_PINNED);
This should be CURSEG_COLD_DATA. Otherwise it causes the below kernel panic.
I fixed this in the -dev, so let me know, if you have other concern.
259 Unable to handle kernel NULL pointer dereference at virtual address 00000008
259 task: 0000000082b4de99 task.stack: 00000000c6b39dbf
259 pc : f2fs_do_write_data_page+0x2b4/0x794
259 lr : f2fs_do_write_data_page+0x290/0x794
259 sp : ffffff800c83b5a0 pstate : 60c00145
259 Call trace:
259 f2fs_do_write_data_page+0x2b4/0x794
259 f2fs_write_single_data_page+0x4a4/0x764
259 f2fs_write_data_pages+0x4dc/0x968
259 do_writepages+0x60/0x124
259 __writeback_single_inode+0xd8/0x490
259 writeback_sb_inodes+0x3a8/0x6e4
259 __writeback_inodes_wb+0xa4/0x14c
259 wb_writeback+0x218/0x434
259 wb_workfn+0x2bc/0x57c
259 process_one_work+0x25c/0x440
259 worker_thread+0x24c/0x480
259 kthread+0x11c/0x12c
259 ret_from_fork+0x10/0x18
> f2fs_unlock_op(sbi);
>
> err = f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_DIO);
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index ae5310f02e7f..af974ba273b3 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -742,7 +742,7 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *inode_list,
> f2fs_put_page(page, 1);
> }
> if (!err)
> - f2fs_allocate_new_segments(sbi, NO_CHECK_TYPE);
> + f2fs_allocate_new_segments(sbi);
> return err;
> }
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 113114f98087..f15711e8ee5b 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2707,28 +2707,35 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> up_read(&SM_I(sbi)->curseg_lock);
> }
>
> -void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi, int type)
> +void __allocate_new_segment(struct f2fs_sb_info *sbi, int type)
> {
> - struct curseg_info *curseg;
> + struct curseg_info *curseg = CURSEG_I(sbi, type);
> unsigned int old_segno;
> - int i;
>
> - down_write(&SIT_I(sbi)->sentry_lock);
> + if (!curseg->next_blkoff &&
> + !get_valid_blocks(sbi, curseg->segno, false) &&
> + !get_ckpt_valid_blocks(sbi, curseg->segno))
> + return;
>
> - for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) {
> - if (type != NO_CHECK_TYPE && i != type)
> - continue;
> + old_segno = curseg->segno;
> + SIT_I(sbi)->s_ops->allocate_segment(sbi, type, true);
> + locate_dirty_segment(sbi, old_segno);
> +}
>
> - curseg = CURSEG_I(sbi, i);
> - if (type == NO_CHECK_TYPE || curseg->next_blkoff ||
> - get_valid_blocks(sbi, curseg->segno, false) ||
> - get_ckpt_valid_blocks(sbi, curseg->segno)) {
> - old_segno = curseg->segno;
> - SIT_I(sbi)->s_ops->allocate_segment(sbi, i, true);
> - locate_dirty_segment(sbi, old_segno);
> - }
> - }
> +void f2fs_allocate_new_segment(struct f2fs_sb_info *sbi, int type)
> +{
> + down_write(&SIT_I(sbi)->sentry_lock);
> + __allocate_new_segment(sbi, type);
> + up_write(&SIT_I(sbi)->sentry_lock);
> +}
>
> +void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi)
> +{
> + int i;
> +
> + down_write(&SIT_I(sbi)->sentry_lock);
> + for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++)
> + __allocate_new_segment(sbi, i);
> up_write(&SIT_I(sbi)->sentry_lock);
> }
>
> --
> 2.26.2
_______________________________________________
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 <yuchao0@huawei.com>
Cc: linux-f2fs-devel@lists.sourceforge.net,
linux-kernel@vger.kernel.org, chao@kernel.org
Subject: Re: [PATCH 1/2] f2fs: split f2fs_allocate_new_segments()
Date: Mon, 29 Jun 2020 13:19:43 -0700 [thread overview]
Message-ID: <20200629201943.GB1117827@google.com> (raw)
In-Reply-To: <20200622093849.35684-1-yuchao0@huawei.com>
On 06/22, Chao Yu wrote:
> to two independent functions:
> - f2fs_allocate_new_segment() for specified type segment allocation
> - f2fs_allocate_new_segments() for all data type segments allocation
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/f2fs.h | 3 ++-
> fs/f2fs/file.c | 2 +-
> fs/f2fs/recovery.c | 2 +-
> fs/f2fs/segment.c | 39 +++++++++++++++++++++++----------------
> 4 files changed, 27 insertions(+), 19 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 70565d81320b..07290943e91d 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3327,7 +3327,8 @@ void f2fs_release_discard_addrs(struct f2fs_sb_info *sbi);
> int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra);
> void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> unsigned int start, unsigned int end);
> -void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi, int type);
> +void f2fs_allocate_new_segment(struct f2fs_sb_info *sbi, int type);
> +void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi);
> int f2fs_trim_fs(struct f2fs_sb_info *sbi, struct fstrim_range *range);
> bool f2fs_exist_trim_candidates(struct f2fs_sb_info *sbi,
> struct cp_control *cpc);
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index f196187159e9..67c65e40b22b 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -1659,7 +1659,7 @@ static int expand_inode_data(struct inode *inode, loff_t offset,
> map.m_seg_type = CURSEG_COLD_DATA_PINNED;
>
> f2fs_lock_op(sbi);
> - f2fs_allocate_new_segments(sbi, CURSEG_COLD_DATA);
> + f2fs_allocate_new_segment(sbi, CURSEG_COLD_DATA_PINNED);
This should be CURSEG_COLD_DATA. Otherwise it causes the below kernel panic.
I fixed this in the -dev, so let me know, if you have other concern.
259 Unable to handle kernel NULL pointer dereference at virtual address 00000008
259 task: 0000000082b4de99 task.stack: 00000000c6b39dbf
259 pc : f2fs_do_write_data_page+0x2b4/0x794
259 lr : f2fs_do_write_data_page+0x290/0x794
259 sp : ffffff800c83b5a0 pstate : 60c00145
259 Call trace:
259 f2fs_do_write_data_page+0x2b4/0x794
259 f2fs_write_single_data_page+0x4a4/0x764
259 f2fs_write_data_pages+0x4dc/0x968
259 do_writepages+0x60/0x124
259 __writeback_single_inode+0xd8/0x490
259 writeback_sb_inodes+0x3a8/0x6e4
259 __writeback_inodes_wb+0xa4/0x14c
259 wb_writeback+0x218/0x434
259 wb_workfn+0x2bc/0x57c
259 process_one_work+0x25c/0x440
259 worker_thread+0x24c/0x480
259 kthread+0x11c/0x12c
259 ret_from_fork+0x10/0x18
> f2fs_unlock_op(sbi);
>
> err = f2fs_map_blocks(inode, &map, 1, F2FS_GET_BLOCK_PRE_DIO);
> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> index ae5310f02e7f..af974ba273b3 100644
> --- a/fs/f2fs/recovery.c
> +++ b/fs/f2fs/recovery.c
> @@ -742,7 +742,7 @@ static int recover_data(struct f2fs_sb_info *sbi, struct list_head *inode_list,
> f2fs_put_page(page, 1);
> }
> if (!err)
> - f2fs_allocate_new_segments(sbi, NO_CHECK_TYPE);
> + f2fs_allocate_new_segments(sbi);
> return err;
> }
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 113114f98087..f15711e8ee5b 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2707,28 +2707,35 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> up_read(&SM_I(sbi)->curseg_lock);
> }
>
> -void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi, int type)
> +void __allocate_new_segment(struct f2fs_sb_info *sbi, int type)
> {
> - struct curseg_info *curseg;
> + struct curseg_info *curseg = CURSEG_I(sbi, type);
> unsigned int old_segno;
> - int i;
>
> - down_write(&SIT_I(sbi)->sentry_lock);
> + if (!curseg->next_blkoff &&
> + !get_valid_blocks(sbi, curseg->segno, false) &&
> + !get_ckpt_valid_blocks(sbi, curseg->segno))
> + return;
>
> - for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) {
> - if (type != NO_CHECK_TYPE && i != type)
> - continue;
> + old_segno = curseg->segno;
> + SIT_I(sbi)->s_ops->allocate_segment(sbi, type, true);
> + locate_dirty_segment(sbi, old_segno);
> +}
>
> - curseg = CURSEG_I(sbi, i);
> - if (type == NO_CHECK_TYPE || curseg->next_blkoff ||
> - get_valid_blocks(sbi, curseg->segno, false) ||
> - get_ckpt_valid_blocks(sbi, curseg->segno)) {
> - old_segno = curseg->segno;
> - SIT_I(sbi)->s_ops->allocate_segment(sbi, i, true);
> - locate_dirty_segment(sbi, old_segno);
> - }
> - }
> +void f2fs_allocate_new_segment(struct f2fs_sb_info *sbi, int type)
> +{
> + down_write(&SIT_I(sbi)->sentry_lock);
> + __allocate_new_segment(sbi, type);
> + up_write(&SIT_I(sbi)->sentry_lock);
> +}
>
> +void f2fs_allocate_new_segments(struct f2fs_sb_info *sbi)
> +{
> + int i;
> +
> + down_write(&SIT_I(sbi)->sentry_lock);
> + for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++)
> + __allocate_new_segment(sbi, i);
> up_write(&SIT_I(sbi)->sentry_lock);
> }
>
> --
> 2.26.2
next prev parent reply other threads:[~2020-06-29 20:20 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-22 9:38 [f2fs-dev] [PATCH 1/2] f2fs: split f2fs_allocate_new_segments() Chao Yu
2020-06-22 9:38 ` Chao Yu
2020-06-22 9:38 ` [f2fs-dev] [PATCH 2/2] f2fs: introduce inmem curseg Chao Yu
2020-06-22 9:38 ` Chao Yu
2020-06-29 20:19 ` Jaegeuk Kim [this message]
2020-06-29 20:19 ` [PATCH 1/2] f2fs: split f2fs_allocate_new_segments() Jaegeuk Kim
2020-06-30 1:21 ` [f2fs-dev] " Chao Yu
2020-06-30 1:21 ` Chao Yu
2020-07-01 1:31 ` [f2fs-dev] " Chao Yu
2020-07-01 1:31 ` Chao Yu
2020-07-01 4:22 ` [f2fs-dev] " Jaegeuk Kim
2020-07-01 4:22 ` 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=20200629201943.GB1117827@google.com \
--to=jaegeuk@kernel.org \
--cc=linux-f2fs-devel@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=yuchao0@huawei.com \
/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.