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: Tue, 30 Jun 2020 21:22:22 -0700 [thread overview]
Message-ID: <20200701042222.GA1539525@google.com> (raw)
In-Reply-To: <0e3c7fe2-9cb8-c457-e251-eb0fb7c0760e@huawei.com>
On 07/01, Chao Yu wrote:
> Jaegeuk, could you please help to change __allocate_new_segment() to static
> in your tree?
Sure. :)
>
> On 2020/6/30 4:19, Jaegeuk Kim wrote:
> > 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: Tue, 30 Jun 2020 21:22:22 -0700 [thread overview]
Message-ID: <20200701042222.GA1539525@google.com> (raw)
In-Reply-To: <0e3c7fe2-9cb8-c457-e251-eb0fb7c0760e@huawei.com>
On 07/01, Chao Yu wrote:
> Jaegeuk, could you please help to change __allocate_new_segment() to static
> in your tree?
Sure. :)
>
> On 2020/6/30 4:19, Jaegeuk Kim wrote:
> > 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-07-01 4:22 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 ` [f2fs-dev] [PATCH 1/2] f2fs: split f2fs_allocate_new_segments() Jaegeuk Kim
2020-06-29 20:19 ` 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 ` Jaegeuk Kim [this message]
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=20200701042222.GA1539525@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.