Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1548686 > unrolled thread

[PATCH 01/10] f2fs: reassign new segment for mode=lfs

Back to article view | Back to linux.kernel

Contents

  [PATCH 01/10] f2fs: reassign new segment for mode=lfs Jaegeuk Kim <jaegeuk@kernel.org> - 2016-12-30 20:00 +0100
    [PATCH 08/10] f2fs: relax async discard commands more Jaegeuk Kim <jaegeuk@kernel.org> - 2016-12-30 20:00 +0100
      Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more Chao Yu <yuchao0@huawei.com> - 2017-01-04 10:40 +0100
        Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more Chao Yu <yuchao0@huawei.com> - 2017-01-05 04:30 +0100
          Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more Chao Yu <yuchao0@huawei.com> - 2017-01-05 10:00 +0100
            Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more Jaegeuk Kim <jaegeuk@kernel.org> - 2017-01-05 21:10 +0100
          Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more Jaegeuk Kim <jaegeuk@kernel.org> - 2017-01-05 20:50 +0100
            Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more Chao Yu <yuchao0@huawei.com> - 2017-01-06 03:10 +0100
              Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more Jaegeuk Kim <jaegeuk@kernel.org> - 2017-01-06 03:50 +0100
                Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more Chao Yu <yuchao0@huawei.com> - 2017-01-06 04:40 +0100
    [PATCH 09/10] f2fs: avoid needless checkpoint in f2fs_trim_fs Jaegeuk Kim <jaegeuk@kernel.org> - 2016-12-30 20:00 +0100
    [PATCH 03/10] f2fs: add submit_bio tracepoint Jaegeuk Kim <jaegeuk@kernel.org> - 2016-12-30 20:00 +0100
      Re: [f2fs-dev] [PATCH 03/10] f2fs: add submit_bio tracepoint Chao Yu <yuchao0@huawei.com> - 2017-01-04 04:40 +0100
      Re: [PATCH 03/10 v2] f2fs: add submit_bio tracepoint Jaegeuk Kim <jaegeuk@kernel.org> - 2017-01-05 00:40 +0100
        Re: [PATCH 03/10 v2] f2fs: add submit_bio tracepoint Chao Yu <yuchao0@huawei.com> - 2017-01-12 12:10 +0100
    [PATCH 02/10] f2fs: fix wrong tracepoints for op and op_flags Jaegeuk Kim <jaegeuk@kernel.org> - 2016-12-30 20:00 +0100
      Re: [f2fs-dev] [PATCH 02/10] f2fs: fix wrong tracepoints for op and  op_flags Chao Yu <yuchao0@huawei.com> - 2017-01-04 04:30 +0100
    Re: [f2fs-dev] [PATCH 01/10] f2fs: reassign new segment for mode=lfs Chao Yu <yuchao0@huawei.com> - 2017-01-04 04:30 +0100
      Re: [f2fs-dev] [PATCH 01/10] f2fs: reassign new segment for mode=lfs Jaegeuk Kim <jaegeuk@kernel.org> - 2017-01-04 23:50 +0100
        Re: [f2fs-dev] [PATCH 01/10] f2fs: reassign new segment for mode=lfs Chao Yu <yuchao0@huawei.com> - 2017-01-12 12:10 +0100

#1548686 — [PATCH 01/10] f2fs: reassign new segment for mode=lfs

Otherwise we can remain wrong curseg->next_blkoff, resulting in fsck failure.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/segment.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index be9e4d244d75..4e5ffe1d97e4 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1428,9 +1428,6 @@ void allocate_new_segments(struct f2fs_sb_info *sbi)
 	unsigned int old_segno;
 	int i;
 
-	if (test_opt(sbi, LFS))
-		return;
-
 	for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) {
 		curseg = CURSEG_I(sbi, i);
 		old_segno = curseg->segno;
-- 
2.11.0

[toc] | [next] | [standalone]

#1548687 — [PATCH 08/10] f2fs: relax async discard commands more

This patch relaxes async discard commands to avoid waiting its end_io during
checkpoint.
Instead of waiting them during checkpoint, it will be done when actually reusing
them.

Test on initial partition of nvme drive.

 # time fstrim /mnt/test

Before : 6.158s
After : 4.822s

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c |  3 ---
 fs/f2fs/f2fs.h       |  4 +++-
 fs/f2fs/segment.c    | 25 +++++++++++++++++++++----
 fs/f2fs/super.c      |  3 +++
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 34bfe2b494ae..1a9ba69a22ba 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1254,7 +1254,6 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 		f2fs_bug_on(sbi, prefree_segments(sbi));
 		flush_sit_entries(sbi, cpc);
 		clear_prefree_segments(sbi, cpc);
-		f2fs_wait_all_discard_bio(sbi);
 		unblock_operations(sbi);
 		goto out;
 	}
@@ -1278,8 +1277,6 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	else
 		clear_prefree_segments(sbi, cpc);
 
-	f2fs_wait_all_discard_bio(sbi);
-
 	unblock_operations(sbi);
 	stat_inc_cp_count(sbi->stat_info);
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index bdcfe2a9b532..f2f40fce9d31 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -183,6 +183,8 @@ struct discard_entry {
 
 struct bio_entry {
 	struct list_head list;
+	unsigned int start_segno;
+	unsigned int end_segno;
 	struct bio *bio;
 	struct completion event;
 	int error;
@@ -2111,7 +2113,7 @@ void destroy_flush_cmd_control(struct f2fs_sb_info *, bool);
 void invalidate_blocks(struct f2fs_sb_info *, block_t);
 bool is_checkpointed_data(struct f2fs_sb_info *, block_t);
 void refresh_sit_entry(struct f2fs_sb_info *, block_t, block_t);
-void f2fs_wait_all_discard_bio(struct f2fs_sb_info *);
+void f2fs_wait_discard_bio(struct f2fs_sb_info *, unsigned int);
 void clear_prefree_segments(struct f2fs_sb_info *, struct cp_control *);
 void release_discard_addrs(struct f2fs_sb_info *);
 int npages_for_summary_flush(struct f2fs_sb_info *, bool);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 6d197f0c8151..9a38424c3c1f 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -624,20 +624,24 @@ static void locate_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno)
 }
 
 static struct bio_entry *__add_bio_entry(struct f2fs_sb_info *sbi,
-							struct bio *bio)
+			struct bio *bio, unsigned int start_segno,
+					unsigned int end_segno)
 {
 	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
 	struct bio_entry *be = f2fs_kmem_cache_alloc(bio_entry_slab, GFP_NOFS);
 
 	INIT_LIST_HEAD(&be->list);
 	be->bio = bio;
+	be->start_segno = start_segno;
+	be->end_segno = end_segno;
 	init_completion(&be->event);
 	list_add_tail(&be->list, wait_list);
 
 	return be;
 }
 
-void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
+/* This should be covered by global mutex, &sit_i->sentry_lock */
+void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
 {
 	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
 	struct bio_entry *be, *tmp;
@@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
 		struct bio *bio = be->bio;
 		int err;
 
-		wait_for_completion_io(&be->event);
+		if (!completion_done(&be->event)) {
+			if ((be->start_segno >= segno &&
+					be->end_segno <= segno) ||
+					segno == NULL_SEGNO)
+				wait_for_completion_io(&be->event);
+			else
+				continue;
+		}
+
 		err = be->error;
 		if (err == -EOPNOTSUPP)
 			err = 0;
@@ -674,6 +686,8 @@ static int __f2fs_issue_discard_async(struct f2fs_sb_info *sbi,
 		struct block_device *bdev, block_t blkstart, block_t blklen)
 {
 	struct bio *bio = NULL;
+	unsigned int start_segno = GET_SEGNO(sbi, blkstart);
+	unsigned int end_segno = GET_SEGNO(sbi, blkstart + blklen);
 	int err;
 
 	trace_f2fs_issue_discard(sbi->sb, blkstart, blklen);
@@ -688,7 +702,8 @@ static int __f2fs_issue_discard_async(struct f2fs_sb_info *sbi,
 				SECTOR_FROM_BLOCK(blklen),
 				GFP_NOFS, 0, &bio);
 	if (!err && bio) {
-		struct bio_entry *be = __add_bio_entry(sbi, bio);
+		struct bio_entry *be = __add_bio_entry(sbi, bio,
+					start_segno, end_segno);
 
 		bio->bi_private = be;
 		bio->bi_end_io = f2fs_submit_bio_wait_endio;
@@ -1574,6 +1589,8 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 
 	*new_blkaddr = NEXT_FREE_BLKADDR(sbi, curseg);
 
+	f2fs_wait_discard_bio(sbi, GET_SEGNO(sbi, *new_blkaddr));
+
 	/*
 	 * __add_sum_entry should be resided under the curseg_mutex
 	 * because, this function updates a summary entry in the
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index f3697f97e527..16e2bc5209bb 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -770,6 +770,9 @@ static void f2fs_put_super(struct super_block *sb)
 		write_checkpoint(sbi, &cpc);
 	}
 
+	/* be sure to wait for any on-going discard commands */
+	f2fs_wait_discard_bio(sbi, NULL_SEGNO);
+
 	/* write_checkpoint can update stat informaion */
 	f2fs_destroy_stats(sbi);
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1550575 — Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more

On 2016/12/31 2:51, Jaegeuk Kim wrote:
> This patch relaxes async discard commands to avoid waiting its end_io during
> checkpoint.
> Instead of waiting them during checkpoint, it will be done when actually reusing
> them.
> 
> Test on initial partition of nvme drive.
> 
>  # time fstrim /mnt/test
> 
> Before : 6.158s
> After : 4.822s
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

One comment below,

> ---
>  fs/f2fs/checkpoint.c |  3 ---
>  fs/f2fs/f2fs.h       |  4 +++-
>  fs/f2fs/segment.c    | 25 +++++++++++++++++++++----
>  fs/f2fs/super.c      |  3 +++
>  4 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 34bfe2b494ae..1a9ba69a22ba 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1254,7 +1254,6 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  		f2fs_bug_on(sbi, prefree_segments(sbi));
>  		flush_sit_entries(sbi, cpc);
>  		clear_prefree_segments(sbi, cpc);
> -		f2fs_wait_all_discard_bio(sbi);
>  		unblock_operations(sbi);
>  		goto out;
>  	}
> @@ -1278,8 +1277,6 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  	else
>  		clear_prefree_segments(sbi, cpc);
>  
> -	f2fs_wait_all_discard_bio(sbi);
> -
>  	unblock_operations(sbi);
>  	stat_inc_cp_count(sbi->stat_info);
>  
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index bdcfe2a9b532..f2f40fce9d31 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -183,6 +183,8 @@ struct discard_entry {
>  
>  struct bio_entry {
>  	struct list_head list;
> +	unsigned int start_segno;
> +	unsigned int end_segno;
>  	struct bio *bio;
>  	struct completion event;
>  	int error;
> @@ -2111,7 +2113,7 @@ void destroy_flush_cmd_control(struct f2fs_sb_info *, bool);
>  void invalidate_blocks(struct f2fs_sb_info *, block_t);
>  bool is_checkpointed_data(struct f2fs_sb_info *, block_t);
>  void refresh_sit_entry(struct f2fs_sb_info *, block_t, block_t);
> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *);
> +void f2fs_wait_discard_bio(struct f2fs_sb_info *, unsigned int);
>  void clear_prefree_segments(struct f2fs_sb_info *, struct cp_control *);
>  void release_discard_addrs(struct f2fs_sb_info *);
>  int npages_for_summary_flush(struct f2fs_sb_info *, bool);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 6d197f0c8151..9a38424c3c1f 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -624,20 +624,24 @@ static void locate_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno)
>  }
>  
>  static struct bio_entry *__add_bio_entry(struct f2fs_sb_info *sbi,
> -							struct bio *bio)
> +			struct bio *bio, unsigned int start_segno,
> +					unsigned int end_segno)
>  {
>  	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
>  	struct bio_entry *be = f2fs_kmem_cache_alloc(bio_entry_slab, GFP_NOFS);
>  
>  	INIT_LIST_HEAD(&be->list);
>  	be->bio = bio;
> +	be->start_segno = start_segno;
> +	be->end_segno = end_segno;
>  	init_completion(&be->event);
>  	list_add_tail(&be->list, wait_list);
>  
>  	return be;
>  }
>  
> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
> +/* This should be covered by global mutex, &sit_i->sentry_lock */
> +void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
>  {
>  	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
>  	struct bio_entry *be, *tmp;
> @@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>  		struct bio *bio = be->bio;
>  		int err;
>  
> -		wait_for_completion_io(&be->event);
> +		if (!completion_done(&be->event)) {
> +			if ((be->start_segno >= segno &&
> +					be->end_segno <= segno) ||

segno >= be->start_segno && segno < be->end_segno ?

Thanks,

> +					segno == NULL_SEGNO)
> +				wait_for_completion_io(&be->event);
> +			else
> +				continue;
> +		}
> +
>  		err = be->error;
>  		if (err == -EOPNOTSUPP)
>  			err = 0;
> @@ -674,6 +686,8 @@ static int __f2fs_issue_discard_async(struct f2fs_sb_info *sbi,
>  		struct block_device *bdev, block_t blkstart, block_t blklen)
>  {
>  	struct bio *bio = NULL;
> +	unsigned int start_segno = GET_SEGNO(sbi, blkstart);
> +	unsigned int end_segno = GET_SEGNO(sbi, blkstart + blklen);
>  	int err;
>  
>  	trace_f2fs_issue_discard(sbi->sb, blkstart, blklen);
> @@ -688,7 +702,8 @@ static int __f2fs_issue_discard_async(struct f2fs_sb_info *sbi,
>  				SECTOR_FROM_BLOCK(blklen),
>  				GFP_NOFS, 0, &bio);
>  	if (!err && bio) {
> -		struct bio_entry *be = __add_bio_entry(sbi, bio);
> +		struct bio_entry *be = __add_bio_entry(sbi, bio,
> +					start_segno, end_segno);
>  
>  		bio->bi_private = be;
>  		bio->bi_end_io = f2fs_submit_bio_wait_endio;
> @@ -1574,6 +1589,8 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>  
>  	*new_blkaddr = NEXT_FREE_BLKADDR(sbi, curseg);
>  
> +	f2fs_wait_discard_bio(sbi, GET_SEGNO(sbi, *new_blkaddr));
> +
>  	/*
>  	 * __add_sum_entry should be resided under the curseg_mutex
>  	 * because, this function updates a summary entry in the
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index f3697f97e527..16e2bc5209bb 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -770,6 +770,9 @@ static void f2fs_put_super(struct super_block *sb)
>  		write_checkpoint(sbi, &cpc);
>  	}
>  
> +	/* be sure to wait for any on-going discard commands */
> +	f2fs_wait_discard_bio(sbi, NULL_SEGNO);
> +
>  	/* write_checkpoint can update stat informaion */
>  	f2fs_destroy_stats(sbi);
>  
> 

[toc] | [prev] | [next] | [standalone]

#1551593 — Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more

On 2017/1/4 17:29, Chao Yu wrote:
> On 2016/12/31 2:51, Jaegeuk Kim wrote:
>> This patch relaxes async discard commands to avoid waiting its end_io during
>> checkpoint.
>> Instead of waiting them during checkpoint, it will be done when actually reusing
>> them.
>>
>> Test on initial partition of nvme drive.
>>
>>  # time fstrim /mnt/test
>>
>> Before : 6.158s
>> After : 4.822s
>>
>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 
> One comment below,

I still have a comment on this patch.

>> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>> +/* This should be covered by global mutex, &sit_i->sentry_lock */
>> +void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
>>  {
>>  	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
>>  	struct bio_entry *be, *tmp;
>> @@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>>  		struct bio *bio = be->bio;
>>  		int err;
>>  
>> -		wait_for_completion_io(&be->event);
>> +		if (!completion_done(&be->event)) {
>> +			if ((be->start_segno >= segno &&
>> +					be->end_segno <= segno) ||
> 
> segno >= be->start_segno && segno < be->end_segno ?

Can you check this?

Thanks,

[toc] | [prev] | [next] | [standalone]

#1551770 — Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more

Hi Jaegeuk,

I can see patch named ("f2fs: call f2fs_wait_all_discard_bio for an error case")
was merged in dev-test, but I think it's no needed to change error case handling
like this since f2fs_wait_all_discard_bio should always be called after
clear_prefree_segments.

Thanks,

On 2017/1/5 11:19, Chao Yu wrote:
> On 2017/1/4 17:29, Chao Yu wrote:
>> On 2016/12/31 2:51, Jaegeuk Kim wrote:
>>> This patch relaxes async discard commands to avoid waiting its end_io during
>>> checkpoint.
>>> Instead of waiting them during checkpoint, it will be done when actually reusing
>>> them.
>>>
>>> Test on initial partition of nvme drive.
>>>
>>>  # time fstrim /mnt/test
>>>
>>> Before : 6.158s
>>> After : 4.822s
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>
>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
>>
>> One comment below,
> 
> I still have a comment on this patch.
> 
>>> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>>> +/* This should be covered by global mutex, &sit_i->sentry_lock */
>>> +void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
>>>  {
>>>  	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
>>>  	struct bio_entry *be, *tmp;
>>> @@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>>>  		struct bio *bio = be->bio;
>>>  		int err;
>>>  
>>> -		wait_for_completion_io(&be->event);
>>> +		if (!completion_done(&be->event)) {
>>> +			if ((be->start_segno >= segno &&
>>> +					be->end_segno <= segno) ||
>>
>> segno >= be->start_segno && segno < be->end_segno ?
> 
> Can you check this?
> 
> Thanks,
> 

[toc] | [prev] | [next] | [standalone]

#1552293 — Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more

Hi Chao,

On 01/05, Chao Yu wrote:
> Hi Jaegeuk,
> 
> I can see patch named ("f2fs: call f2fs_wait_all_discard_bio for an error case")
> was merged in dev-test, but I think it's no needed to change error case handling
> like this since f2fs_wait_all_discard_bio should always be called after
> clear_prefree_segments.

Indeed, it's right. ;)

Thanks,

> 
> Thanks,
> 
> On 2017/1/5 11:19, Chao Yu wrote:
> > On 2017/1/4 17:29, Chao Yu wrote:
> >> On 2016/12/31 2:51, Jaegeuk Kim wrote:
> >>> This patch relaxes async discard commands to avoid waiting its end_io during
> >>> checkpoint.
> >>> Instead of waiting them during checkpoint, it will be done when actually reusing
> >>> them.
> >>>
> >>> Test on initial partition of nvme drive.
> >>>
> >>>  # time fstrim /mnt/test
> >>>
> >>> Before : 6.158s
> >>> After : 4.822s
> >>>
> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> >>
> >> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> >>
> >> One comment below,
> > 
> > I still have a comment on this patch.
> > 
> >>> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
> >>> +/* This should be covered by global mutex, &sit_i->sentry_lock */
> >>> +void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
> >>>  {
> >>>  	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
> >>>  	struct bio_entry *be, *tmp;
> >>> @@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
> >>>  		struct bio *bio = be->bio;
> >>>  		int err;
> >>>  
> >>> -		wait_for_completion_io(&be->event);
> >>> +		if (!completion_done(&be->event)) {
> >>> +			if ((be->start_segno >= segno &&
> >>> +					be->end_segno <= segno) ||
> >>
> >> segno >= be->start_segno && segno < be->end_segno ?
> > 
> > Can you check this?
> > 
> > Thanks,
> > 

[toc] | [prev] | [next] | [standalone]

#1552287 — Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more

On 01/05, Chao Yu wrote:
> On 2017/1/4 17:29, Chao Yu wrote:
> > On 2016/12/31 2:51, Jaegeuk Kim wrote:
> >> This patch relaxes async discard commands to avoid waiting its end_io during
> >> checkpoint.
> >> Instead of waiting them during checkpoint, it will be done when actually reusing
> >> them.
> >>
> >> Test on initial partition of nvme drive.
> >>
> >>  # time fstrim /mnt/test
> >>
> >> Before : 6.158s
> >> After : 4.822s
> >>
> >> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > 
> > Reviewed-by: Chao Yu <yuchao0@huawei.com>
> > 
> > One comment below,
> 
> I still have a comment on this patch.
> 
> >> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
> >> +/* This should be covered by global mutex, &sit_i->sentry_lock */
> >> +void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
> >>  {
> >>  	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
> >>  	struct bio_entry *be, *tmp;
> >> @@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
> >>  		struct bio *bio = be->bio;
> >>  		int err;
> >>  
> >> -		wait_for_completion_io(&be->event);
> >> +		if (!completion_done(&be->event)) {
> >> +			if ((be->start_segno >= segno &&
> >> +					be->end_segno <= segno) ||
> > 
> > segno >= be->start_segno && segno < be->end_segno ?
> 
> Can you check this?

The be->start_segno and be->end_segno are assigned by:

unsigned int start_segno = GET_SEGNO(sbi, blkstart);
unsigned int end_segno = GET_SEGNO(sbi, blkstart + blklen);

in __f2fs_issue_discard_async().

So, the problem comes when, for example, blkstart = 0 and blklen = 512.

I should have got the end_segno by:

unsigned int end_segno = GET_SEGNO(sbi, blkstart + blklen - 1);

Thanks,

[toc] | [prev] | [next] | [standalone]

#1552472 — Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more

On 2017/1/6 3:46, Jaegeuk Kim wrote:
> On 01/05, Chao Yu wrote:
>> On 2017/1/4 17:29, Chao Yu wrote:
>>> On 2016/12/31 2:51, Jaegeuk Kim wrote:
>>>> This patch relaxes async discard commands to avoid waiting its end_io during
>>>> checkpoint.
>>>> Instead of waiting them during checkpoint, it will be done when actually reusing
>>>> them.
>>>>
>>>> Test on initial partition of nvme drive.
>>>>
>>>>  # time fstrim /mnt/test
>>>>
>>>> Before : 6.158s
>>>> After : 4.822s
>>>>
>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>
>>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
>>>
>>> One comment below,
>>
>> I still have a comment on this patch.
>>
>>>> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>>>> +/* This should be covered by global mutex, &sit_i->sentry_lock */
>>>> +void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
>>>>  {
>>>>  	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
>>>>  	struct bio_entry *be, *tmp;
>>>> @@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>>>>  		struct bio *bio = be->bio;
>>>>  		int err;
>>>>  
>>>> -		wait_for_completion_io(&be->event);
>>>> +		if (!completion_done(&be->event)) {
>>>> +			if ((be->start_segno >= segno &&
>>>> +					be->end_segno <= segno) ||
>>>
>>> segno >= be->start_segno && segno < be->end_segno ?

Still can not understand this judgment condition, we should wait completion of
discard command only when segno is locate in range of [start_segno, end_segno]?

But now, this condition can be true only when segno, start_segno, end_segno have
equal value.

Please correct me if I'm wrong.

Thanks,

>>
>> Can you check this?
> 
> The be->start_segno and be->end_segno are assigned by:
> 
> unsigned int start_segno = GET_SEGNO(sbi, blkstart);
> unsigned int end_segno = GET_SEGNO(sbi, blkstart + blklen);
> 
> in __f2fs_issue_discard_async().
> 
> So, the problem comes when, for example, blkstart = 0 and blklen = 512.
> 
> I should have got the end_segno by:
> 
> unsigned int end_segno = GET_SEGNO(sbi, blkstart + blklen - 1);
> 
> Thanks,
> 
> .
> 

[toc] | [prev] | [next] | [standalone]

#1552484 — Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more

Hi Chao,

On 01/06, Chao Yu wrote:
> On 2017/1/6 3:46, Jaegeuk Kim wrote:
> > On 01/05, Chao Yu wrote:
> >> On 2017/1/4 17:29, Chao Yu wrote:
> >>> On 2016/12/31 2:51, Jaegeuk Kim wrote:
> >>>> This patch relaxes async discard commands to avoid waiting its end_io during
> >>>> checkpoint.
> >>>> Instead of waiting them during checkpoint, it will be done when actually reusing
> >>>> them.
> >>>>
> >>>> Test on initial partition of nvme drive.
> >>>>
> >>>>  # time fstrim /mnt/test
> >>>>
> >>>> Before : 6.158s
> >>>> After : 4.822s
> >>>>
> >>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> >>>
> >>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> >>>
> >>> One comment below,
> >>
> >> I still have a comment on this patch.
> >>
> >>>> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
> >>>> +/* This should be covered by global mutex, &sit_i->sentry_lock */
> >>>> +void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
> >>>>  {
> >>>>  	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
> >>>>  	struct bio_entry *be, *tmp;
> >>>> @@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
> >>>>  		struct bio *bio = be->bio;
> >>>>  		int err;
> >>>>  
> >>>> -		wait_for_completion_io(&be->event);
> >>>> +		if (!completion_done(&be->event)) {
> >>>> +			if ((be->start_segno >= segno &&
> >>>> +					be->end_segno <= segno) ||
> >>>
> >>> segno >= be->start_segno && segno < be->end_segno ?
> 
> Still can not understand this judgment condition, we should wait completion of
> discard command only when segno is locate in range of [start_segno, end_segno]?
> 
> But now, this condition can be true only when segno, start_segno, end_segno have
> equal value.
> 
> Please correct me if I'm wrong.

Urg. I rewrote it to use block addresses.

How about this?

From fe24461eedd62815e0c56317f010a3a6e3004434 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Thu, 29 Dec 2016 14:07:53 -0800
Subject: [PATCH] f2fs: relax async discard commands more

This patch relaxes async discard commands to avoid waiting its end_io during
checkpoint.
Instead of waiting them during checkpoint, it will be done when actually reusing
them.

Test on initial partition of nvme drive.

 # time fstrim /mnt/test

Before : 6.158s
After : 4.822s

Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c |  7 ++-----
 fs/f2fs/f2fs.h       |  4 +++-
 fs/f2fs/segment.c    | 23 +++++++++++++++++++----
 fs/f2fs/super.c      |  3 +++
 4 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index f73ee9534d83..1a9ba69a22ba 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1254,7 +1254,6 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 		f2fs_bug_on(sbi, prefree_segments(sbi));
 		flush_sit_entries(sbi, cpc);
 		clear_prefree_segments(sbi, cpc);
-		f2fs_wait_all_discard_bio(sbi);
 		unblock_operations(sbi);
 		goto out;
 	}
@@ -1273,12 +1272,10 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 
 	/* unlock all the fs_lock[] in do_checkpoint() */
 	err = do_checkpoint(sbi, cpc);
-	if (err) {
+	if (err)
 		release_discard_addrs(sbi);
-	} else {
+	else
 		clear_prefree_segments(sbi, cpc);
-		f2fs_wait_all_discard_bio(sbi);
-	}
 
 	unblock_operations(sbi);
 	stat_inc_cp_count(sbi->stat_info);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index bdcfe2a9b532..e0db895fd84c 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -183,6 +183,8 @@ struct discard_entry {
 
 struct bio_entry {
 	struct list_head list;
+	block_t lstart;
+	block_t len;
 	struct bio *bio;
 	struct completion event;
 	int error;
@@ -2111,7 +2113,7 @@ void destroy_flush_cmd_control(struct f2fs_sb_info *, bool);
 void invalidate_blocks(struct f2fs_sb_info *, block_t);
 bool is_checkpointed_data(struct f2fs_sb_info *, block_t);
 void refresh_sit_entry(struct f2fs_sb_info *, block_t, block_t);
-void f2fs_wait_all_discard_bio(struct f2fs_sb_info *);
+void f2fs_wait_discard_bio(struct f2fs_sb_info *, block_t);
 void clear_prefree_segments(struct f2fs_sb_info *, struct cp_control *);
 void release_discard_addrs(struct f2fs_sb_info *);
 int npages_for_summary_flush(struct f2fs_sb_info *, bool);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 485b9118b418..672bcdefe836 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -625,20 +625,23 @@ static void locate_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno)
 }
 
 static struct bio_entry *__add_bio_entry(struct f2fs_sb_info *sbi,
-							struct bio *bio)
+			struct bio *bio, block_t lstart, block_t len)
 {
 	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
 	struct bio_entry *be = f2fs_kmem_cache_alloc(bio_entry_slab, GFP_NOFS);
 
 	INIT_LIST_HEAD(&be->list);
 	be->bio = bio;
+	be->lstart = lstart;
+	be->len = len;
 	init_completion(&be->event);
 	list_add_tail(&be->list, wait_list);
 
 	return be;
 }
 
-void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
+/* This should be covered by global mutex, &sit_i->sentry_lock */
+void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, block_t blkaddr)
 {
 	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
 	struct bio_entry *be, *tmp;
@@ -647,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
 		struct bio *bio = be->bio;
 		int err;
 
-		wait_for_completion_io(&be->event);
+		if (!completion_done(&be->event)) {
+			if ((be->lstart <= blkaddr &&
+					blkaddr < be->lstart + be->len) ||
+					blkaddr == NULL_ADDR)
+				wait_for_completion_io(&be->event);
+			else
+				continue;
+		}
+
 		err = be->error;
 		if (err == -EOPNOTSUPP)
 			err = 0;
@@ -675,6 +686,7 @@ static int __f2fs_issue_discard_async(struct f2fs_sb_info *sbi,
 		struct block_device *bdev, block_t blkstart, block_t blklen)
 {
 	struct bio *bio = NULL;
+	block_t lblkstart = blkstart;
 	int err;
 
 	trace_f2fs_issue_discard(sbi->sb, blkstart, blklen);
@@ -689,7 +701,8 @@ static int __f2fs_issue_discard_async(struct f2fs_sb_info *sbi,
 				SECTOR_FROM_BLOCK(blklen),
 				GFP_NOFS, 0, &bio);
 	if (!err && bio) {
-		struct bio_entry *be = __add_bio_entry(sbi, bio);
+		struct bio_entry *be = __add_bio_entry(sbi, bio,
+						lblkstart, blklen);
 
 		bio->bi_private = be;
 		bio->bi_end_io = f2fs_submit_bio_wait_endio;
@@ -1575,6 +1588,8 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
 
 	*new_blkaddr = NEXT_FREE_BLKADDR(sbi, curseg);
 
+	f2fs_wait_discard_bio(sbi, *new_blkaddr);
+
 	/*
 	 * __add_sum_entry should be resided under the curseg_mutex
 	 * because, this function updates a summary entry in the
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index f3697f97e527..461c29043aec 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -770,6 +770,9 @@ static void f2fs_put_super(struct super_block *sb)
 		write_checkpoint(sbi, &cpc);
 	}
 
+	/* be sure to wait for any on-going discard commands */
+	f2fs_wait_discard_bio(sbi, NULL_ADDR);
+
 	/* write_checkpoint can update stat informaion */
 	f2fs_destroy_stats(sbi);
 
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1552498 — Re: [f2fs-dev] [PATCH 08/10] f2fs: relax async discard commands more

Hi Jaegeuk,

On 2017/1/6 10:42, Jaegeuk Kim wrote:
> Hi Chao,
> 
> On 01/06, Chao Yu wrote:
>> On 2017/1/6 3:46, Jaegeuk Kim wrote:
>>> On 01/05, Chao Yu wrote:
>>>> On 2017/1/4 17:29, Chao Yu wrote:
>>>>> On 2016/12/31 2:51, Jaegeuk Kim wrote:
>>>>>> This patch relaxes async discard commands to avoid waiting its end_io during
>>>>>> checkpoint.
>>>>>> Instead of waiting them during checkpoint, it will be done when actually reusing
>>>>>> them.
>>>>>>
>>>>>> Test on initial partition of nvme drive.
>>>>>>
>>>>>>  # time fstrim /mnt/test
>>>>>>
>>>>>> Before : 6.158s
>>>>>> After : 4.822s
>>>>>>
>>>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>>>
>>>>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
>>>>>
>>>>> One comment below,
>>>>
>>>> I still have a comment on this patch.
>>>>
>>>>>> -void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>>>>>> +/* This should be covered by global mutex, &sit_i->sentry_lock */
>>>>>> +void f2fs_wait_discard_bio(struct f2fs_sb_info *sbi, unsigned int segno)
>>>>>>  {
>>>>>>  	struct list_head *wait_list = &(SM_I(sbi)->wait_list);
>>>>>>  	struct bio_entry *be, *tmp;
>>>>>> @@ -646,7 +650,15 @@ void f2fs_wait_all_discard_bio(struct f2fs_sb_info *sbi)
>>>>>>  		struct bio *bio = be->bio;
>>>>>>  		int err;
>>>>>>  
>>>>>> -		wait_for_completion_io(&be->event);
>>>>>> +		if (!completion_done(&be->event)) {
>>>>>> +			if ((be->start_segno >= segno &&
>>>>>> +					be->end_segno <= segno) ||
>>>>>
>>>>> segno >= be->start_segno && segno < be->end_segno ?
>>
>> Still can not understand this judgment condition, we should wait completion of
>> discard command only when segno is locate in range of [start_segno, end_segno]?
>>
>> But now, this condition can be true only when segno, start_segno, end_segno have
>> equal value.
>>
>> Please correct me if I'm wrong.
> 
> Urg. I rewrote it to use block addresses.
> 
> How about this?

Looks good to me. Nice work!

Thanks,

[toc] | [prev] | [next] | [standalone]

#1548688 — [PATCH 09/10] f2fs: avoid needless checkpoint in f2fs_trim_fs

The f2fs_trim_fs() doesn't need to do checkpoint if there are newly allocated
data blocks only which didn't change the critical checkpoint data such as nat
and sit entries.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/checkpoint.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 1a9ba69a22ba..917b5c5053ae 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -1248,14 +1248,15 @@ int write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc)
 	f2fs_flush_merged_bios(sbi);
 
 	/* this is the case of multiple fstrims without any changes */
-	if (cpc->reason == CP_DISCARD && !is_sbi_flag_set(sbi, SBI_IS_DIRTY)) {
-		f2fs_bug_on(sbi, NM_I(sbi)->dirty_nat_cnt);
-		f2fs_bug_on(sbi, SIT_I(sbi)->dirty_sentries);
-		f2fs_bug_on(sbi, prefree_segments(sbi));
-		flush_sit_entries(sbi, cpc);
-		clear_prefree_segments(sbi, cpc);
-		unblock_operations(sbi);
-		goto out;
+	if (cpc->reason == CP_DISCARD) {
+		if (NM_I(sbi)->dirty_nat_cnt == 0 &&
+				SIT_I(sbi)->dirty_sentries == 0 &&
+				prefree_segments(sbi) == 0) {
+			flush_sit_entries(sbi, cpc);
+			clear_prefree_segments(sbi, cpc);
+			unblock_operations(sbi);
+			goto out;
+		}
 	}
 
 	/*
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1548689 — [PATCH 03/10] f2fs: add submit_bio tracepoint

This patch adds final submit_bio() tracepoint.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c              |  1 +
 include/trace/events/f2fs.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 2c5df1dc1479..54da4e2f1318 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -175,6 +175,7 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
 			current->plug && (type == DATA || type == NODE))
 			blk_finish_plug(current->plug);
 	}
+	trace_f2fs_submit_bio(sbi->sb, bio);
 	submit_bio(bio);
 }
 
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 4c942599581b..094d92b52450 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -55,6 +55,8 @@ TRACE_DEFINE_ENUM(CP_DISCARD);
 		{ IPU,		"IN-PLACE" },				\
 		{ OPU,		"OUT-OF-PLACE" })
 
+#define F2FS_OP	(REQ_OP_READ | REQ_OP_RAHEAD | REQ_SYNC | REQ_PREFLUSH | REQ_META |\
+			REQ_PRIO)
 #define F2FS_OP_FLAGS (REQ_RAHEAD | REQ_SYNC | REQ_PREFLUSH | REQ_META |\
 			REQ_PRIO)
 #define F2FS_BIO_FLAG_MASK(t)	(t & F2FS_OP_FLAGS)
@@ -837,6 +839,35 @@ DEFINE_EVENT_CONDITION(f2fs__submit_bio, f2fs_submit_read_bio,
 	TP_CONDITION(bio)
 );
 
+TRACE_EVENT(f2fs_submit_bio,
+
+	TP_PROTO(struct super_block *sb, struct bio *bio),
+
+	TP_ARGS(sb, bio),
+
+	TP_STRUCT__entry(
+		__field(dev_t,	dev)
+		__field(int,	op)
+		__field(int,	op_flags)
+		__field(sector_t,	sector)
+		__field(unsigned int,	size)
+	),
+
+	TP_fast_assign(
+		__entry->dev		= sb->s_dev;
+		__entry->op		= bio->bi_opf & REQ_OP_MASK;
+		__entry->op_flags	= bio->bi_opf;
+		__entry->sector		= bio->bi_iter.bi_sector;
+		__entry->size		= bio->bi_iter.bi_size;
+	),
+
+	TP_printk("dev = (%d,%d), rw = %s%s, sector = %lld, size = %u",
+		show_dev(__entry),
+		show_bio_type(__entry->op, __entry->op_flags),
+		(unsigned long long)__entry->sector,
+		__entry->size)
+);
+
 TRACE_EVENT(f2fs_write_begin,
 
 	TP_PROTO(struct inode *inode, loff_t pos, unsigned int len,
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1550413 — Re: [f2fs-dev] [PATCH 03/10] f2fs: add submit_bio tracepoint

Hi Jaegeuk,

On 2016/12/31 2:51, Jaegeuk Kim wrote:
> This patch adds final submit_bio() tracepoint.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/data.c              |  1 +
>  include/trace/events/f2fs.h | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 2c5df1dc1479..54da4e2f1318 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -175,6 +175,7 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
>  			current->plug && (type == DATA || type == NODE))
>  			blk_finish_plug(current->plug);
>  	}
> +	trace_f2fs_submit_bio(sbi->sb, bio);
>  	submit_bio(bio);
>  }
>  
> diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
> index 4c942599581b..094d92b52450 100644
> --- a/include/trace/events/f2fs.h
> +++ b/include/trace/events/f2fs.h
> @@ -55,6 +55,8 @@ TRACE_DEFINE_ENUM(CP_DISCARD);
>  		{ IPU,		"IN-PLACE" },				\
>  		{ OPU,		"OUT-OF-PLACE" })
>  
> +#define F2FS_OP	(REQ_OP_READ | REQ_OP_RAHEAD | REQ_SYNC | REQ_PREFLUSH | REQ_META |\
> +			REQ_PRIO)

Needed?

>  #define F2FS_OP_FLAGS (REQ_RAHEAD | REQ_SYNC | REQ_PREFLUSH | REQ_META |\
>  			REQ_PRIO)
>  #define F2FS_BIO_FLAG_MASK(t)	(t & F2FS_OP_FLAGS)
> @@ -837,6 +839,35 @@ DEFINE_EVENT_CONDITION(f2fs__submit_bio, f2fs_submit_read_bio,
>  	TP_CONDITION(bio)
>  );
>  
> +TRACE_EVENT(f2fs_submit_bio,

Can we reuse f2fs__submit_bio class like f2fs_submit_{read,write}_bio?

DEFINE_EVENT_CONDITION(f2fs__submit_bio, f2fs_submit_last_bio,

Thanks,

> +
> +	TP_PROTO(struct super_block *sb, struct bio *bio),
> +
> +	TP_ARGS(sb, bio),
> +
> +	TP_STRUCT__entry(
> +		__field(dev_t,	dev)
> +		__field(int,	op)
> +		__field(int,	op_flags)
> +		__field(sector_t,	sector)
> +		__field(unsigned int,	size)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->dev		= sb->s_dev;
> +		__entry->op		= bio->bi_opf & REQ_OP_MASK;
> +		__entry->op_flags	= bio->bi_opf;
> +		__entry->sector		= bio->bi_iter.bi_sector;
> +		__entry->size		= bio->bi_iter.bi_size;
> +	),
> +
> +	TP_printk("dev = (%d,%d), rw = %s%s, sector = %lld, size = %u",
> +		show_dev(__entry),
> +		show_bio_type(__entry->op, __entry->op_flags),
> +		(unsigned long long)__entry->sector,
> +		__entry->size)
> +);
> +
>  TRACE_EVENT(f2fs_write_begin,
>  
>  	TP_PROTO(struct inode *inode, loff_t pos, unsigned int len,
> 

[toc] | [prev] | [next] | [standalone]

#1551496 — Re: [PATCH 03/10 v2] f2fs: add submit_bio tracepoint

Change log from v1:
 - remove F2FS_OP
 - share single tracing structure, f2fs__bio
 - split f2fs_prepare_read/write_io and f2fs_submit_read/write_io

From f5c6ab0f19fb627ebf28ab6a18c0a32b4c8f914f Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <jaegeuk@kernel.org>
Date: Wed, 21 Dec 2016 12:13:03 -0800
Subject: [PATCH] f2fs: add submit_bio tracepoint

This patch adds final submit_bio() tracepoint.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/data.c              | 12 ++++++++----
 include/trace/events/f2fs.h | 45 ++++++++++++++++++++++++++++++---------------
 2 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 2c5df1dc1479..a06b2d187aec 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -175,6 +175,10 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
 			current->plug && (type == DATA || type == NODE))
 			blk_finish_plug(current->plug);
 	}
+	if (is_read_io(bio_op(bio)))
+		trace_f2fs_submit_read_bio(sbi->sb, type, bio);
+	else
+		trace_f2fs_submit_write_bio(sbi->sb, type, bio);
 	submit_bio(bio);
 }
 
@@ -185,12 +189,12 @@ static void __submit_merged_bio(struct f2fs_bio_info *io)
 	if (!io->bio)
 		return;
 
+	bio_set_op_attrs(io->bio, fio->op, fio->op_flags);
+
 	if (is_read_io(fio->op))
-		trace_f2fs_submit_read_bio(io->sbi->sb, fio, io->bio);
+		trace_f2fs_prepare_read_bio(io->sbi->sb, fio->type, io->bio);
 	else
-		trace_f2fs_submit_write_bio(io->sbi->sb, fio, io->bio);
-
-	bio_set_op_attrs(io->bio, fio->op, fio->op_flags);
+		trace_f2fs_prepare_write_bio(io->sbi->sb, fio->type, io->bio);
 
 	__submit_bio(io->sbi, io->bio, fio->type);
 	io->bio = NULL;
diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 4c942599581b..04c527410ecc 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -784,12 +784,11 @@ DEFINE_EVENT_CONDITION(f2fs__submit_page_bio, f2fs_submit_page_mbio,
 	TP_CONDITION(page->mapping)
 );
 
-DECLARE_EVENT_CLASS(f2fs__submit_bio,
+DECLARE_EVENT_CLASS(f2fs__bio,
 
-	TP_PROTO(struct super_block *sb, struct f2fs_io_info *fio,
-						struct bio *bio),
+	TP_PROTO(struct super_block *sb, int type, struct bio *bio),
 
-	TP_ARGS(sb, fio, bio),
+	TP_ARGS(sb, type, bio),
 
 	TP_STRUCT__entry(
 		__field(dev_t,	dev)
@@ -802,9 +801,9 @@ DECLARE_EVENT_CLASS(f2fs__submit_bio,
 
 	TP_fast_assign(
 		__entry->dev		= sb->s_dev;
-		__entry->op		= fio->op;
-		__entry->op_flags	= fio->op_flags;
-		__entry->type		= fio->type;
+		__entry->op		= bio_op(bio);
+		__entry->op_flags	= bio->bi_opf;
+		__entry->type		= type;
 		__entry->sector		= bio->bi_iter.bi_sector;
 		__entry->size		= bio->bi_iter.bi_size;
 	),
@@ -817,22 +816,38 @@ DECLARE_EVENT_CLASS(f2fs__submit_bio,
 		__entry->size)
 );
 
-DEFINE_EVENT_CONDITION(f2fs__submit_bio, f2fs_submit_write_bio,
+DEFINE_EVENT_CONDITION(f2fs__bio, f2fs_prepare_write_bio,
+
+	TP_PROTO(struct super_block *sb, int type, struct bio *bio),
+
+	TP_ARGS(sb, type, bio),
+
+	TP_CONDITION(bio)
+);
+
+DEFINE_EVENT_CONDITION(f2fs__bio, f2fs_prepare_read_bio,
+
+	TP_PROTO(struct super_block *sb, int type, struct bio *bio),
+
+	TP_ARGS(sb, type, bio),
+
+	TP_CONDITION(bio)
+);
+
+DEFINE_EVENT_CONDITION(f2fs__bio, f2fs_submit_read_bio,
 
-	TP_PROTO(struct super_block *sb, struct f2fs_io_info *fio,
-							struct bio *bio),
+	TP_PROTO(struct super_block *sb, int type, struct bio *bio),
 
-	TP_ARGS(sb, fio, bio),
+	TP_ARGS(sb, type, bio),
 
 	TP_CONDITION(bio)
 );
 
-DEFINE_EVENT_CONDITION(f2fs__submit_bio, f2fs_submit_read_bio,
+DEFINE_EVENT_CONDITION(f2fs__bio, f2fs_submit_write_bio,
 
-	TP_PROTO(struct super_block *sb, struct f2fs_io_info *fio,
-							struct bio *bio),
+	TP_PROTO(struct super_block *sb, int type, struct bio *bio),
 
-	TP_ARGS(sb, fio, bio),
+	TP_ARGS(sb, type, bio),
 
 	TP_CONDITION(bio)
 );
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1557363 — Re: [PATCH 03/10 v2] f2fs: add submit_bio tracepoint

On 2017/1/5 7:35, Jaegeuk Kim wrote:
> Change log from v1:
>  - remove F2FS_OP
>  - share single tracing structure, f2fs__bio
>  - split f2fs_prepare_read/write_io and f2fs_submit_read/write_io
> 
>>From f5c6ab0f19fb627ebf28ab6a18c0a32b4c8f914f Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <jaegeuk@kernel.org>
> Date: Wed, 21 Dec 2016 12:13:03 -0800
> Subject: [PATCH] f2fs: add submit_bio tracepoint
> 
> This patch adds final submit_bio() tracepoint.

More neat, looks good to me!

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

[toc] | [prev] | [next] | [standalone]

#1548693 — [PATCH 02/10] f2fs: fix wrong tracepoints for op and op_flags

This patch fixes wrong tracepoints in terms of op and op_flags.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 include/trace/events/f2fs.h | 41 +++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/include/trace/events/f2fs.h b/include/trace/events/f2fs.h
index 01b3c9869a0d..4c942599581b 100644
--- a/include/trace/events/f2fs.h
+++ b/include/trace/events/f2fs.h
@@ -55,25 +55,34 @@ TRACE_DEFINE_ENUM(CP_DISCARD);
 		{ IPU,		"IN-PLACE" },				\
 		{ OPU,		"OUT-OF-PLACE" })
 
-#define F2FS_BIO_FLAG_MASK(t)	(t & (REQ_RAHEAD | REQ_PREFLUSH | REQ_FUA))
-#define F2FS_BIO_EXTRA_MASK(t)	(t & (REQ_META | REQ_PRIO))
-
-#define show_bio_type(op_flags)	show_bio_op_flags(op_flags), 		\
-						show_bio_extra(op_flags)
+#define F2FS_OP_FLAGS (REQ_RAHEAD | REQ_SYNC | REQ_PREFLUSH | REQ_META |\
+			REQ_PRIO)
+#define F2FS_BIO_FLAG_MASK(t)	(t & F2FS_OP_FLAGS)
+
+#define show_bio_type(op,op_flags)	show_bio_op(op),		\
+						show_bio_op_flags(op_flags)
+
+#define show_bio_op(op)							\
+	__print_symbolic(op,						\
+		{ REQ_OP_READ,			"READ" },		\
+		{ REQ_OP_WRITE,			"WRITE" },		\
+		{ REQ_OP_FLUSH,			"FLUSH" },		\
+		{ REQ_OP_DISCARD,		"DISCARD" },		\
+		{ REQ_OP_ZONE_REPORT,		"ZONE_REPORT" },	\
+		{ REQ_OP_SECURE_ERASE,		"SECURE_ERASE" },	\
+		{ REQ_OP_ZONE_RESET,		"ZONE_RESET" },		\
+		{ REQ_OP_WRITE_SAME,		"WRITE_SAME" },		\
+		{ REQ_OP_WRITE_ZEROES,		"WRITE_ZEROES" })
 
 #define show_bio_op_flags(flags)					\
 	__print_symbolic(F2FS_BIO_FLAG_MASK(flags),			\
-		{ 0,			"WRITE" },			\
-		{ REQ_RAHEAD, 		"READAHEAD" },			\
-		{ REQ_SYNC, 		"REQ_SYNC" },			\
-		{ REQ_PREFLUSH,		"REQ_PREFLUSH" },		\
-		{ REQ_FUA,		"REQ_FUA" })
-
-#define show_bio_extra(type)						\
-	__print_symbolic(F2FS_BIO_EXTRA_MASK(type),			\
+		{ REQ_RAHEAD, 		"(RA)" },			\
+		{ REQ_SYNC, 		"(S)" },			\
+		{ REQ_SYNC | REQ_PRIO,	"(SP)" },			\
 		{ REQ_META, 		"(M)" },			\
-		{ REQ_PRIO, 		"(P)" },			\
 		{ REQ_META | REQ_PRIO,	"(MP)" },			\
+		{ REQ_SYNC | REQ_META | REQ_PRIO, "(SMP)" },		\
+		{ REQ_PREFLUSH | REQ_META | REQ_PRIO, "(FMP)" },	\
 		{ 0, " \b" })
 
 #define show_data_type(type)						\
@@ -753,7 +762,7 @@ DECLARE_EVENT_CLASS(f2fs__submit_page_bio,
 		(unsigned long)__entry->index,
 		(unsigned long long)__entry->old_blkaddr,
 		(unsigned long long)__entry->new_blkaddr,
-		show_bio_type(__entry->op_flags),
+		show_bio_type(__entry->op, __entry->op_flags),
 		show_block_type(__entry->type))
 );
 
@@ -802,7 +811,7 @@ DECLARE_EVENT_CLASS(f2fs__submit_bio,
 
 	TP_printk("dev = (%d,%d), rw = %s%s, %s, sector = %lld, size = %u",
 		show_dev(__entry),
-		show_bio_type(__entry->op_flags),
+		show_bio_type(__entry->op, __entry->op_flags),
 		show_block_type(__entry->type),
 		(unsigned long long)__entry->sector,
 		__entry->size)
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1550412 — Re: [f2fs-dev] [PATCH 02/10] f2fs: fix wrong tracepoints for op and op_flags

On 2016/12/31 2:51, Jaegeuk Kim wrote:
> This patch fixes wrong tracepoints in terms of op and op_flags.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

[toc] | [prev] | [next] | [standalone]

#1550410 — Re: [f2fs-dev] [PATCH 01/10] f2fs: reassign new segment for mode=lfs

Hi Jaegeuk,

On 2016/12/31 2:51, Jaegeuk Kim wrote:
> Otherwise we can remain wrong curseg->next_blkoff, resulting in fsck failure.

Could you explain more about this case?

Thanks,

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>  fs/f2fs/segment.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index be9e4d244d75..4e5ffe1d97e4 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1428,9 +1428,6 @@ void allocate_new_segments(struct f2fs_sb_info *sbi)
>  	unsigned int old_segno;
>  	int i;
>  
> -	if (test_opt(sbi, LFS))
> -		return;
> -
>  	for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) {
>  		curseg = CURSEG_I(sbi, i);
>  		old_segno = curseg->segno;
> 

[toc] | [prev] | [next] | [standalone]

#1551470 — Re: [f2fs-dev] [PATCH 01/10] f2fs: reassign new segment for mode=lfs

On 01/04, Chao Yu wrote:
> Hi Jaegeuk,
> 
> On 2016/12/31 2:51, Jaegeuk Kim wrote:
> > Otherwise we can remain wrong curseg->next_blkoff, resulting in fsck failure.
> 
> Could you explain more about this case?

I remember that I hit an fsck failure when I was testing f2fs with an smr drive.
I didn't dig into the error, but the fact is that our roll-forward recovery
doesn't update current segment information at every time, but allocate a new
section at the end of the work like below.
I just enabled it for the LFS mode in order to avoid that failure.

Thanks,

> 
> Thanks,
> 
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >  fs/f2fs/segment.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index be9e4d244d75..4e5ffe1d97e4 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1428,9 +1428,6 @@ void allocate_new_segments(struct f2fs_sb_info *sbi)
> >  	unsigned int old_segno;
> >  	int i;
> >  
> > -	if (test_opt(sbi, LFS))
> > -		return;
> > -
> >  	for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) {
> >  		curseg = CURSEG_I(sbi, i);
> >  		old_segno = curseg->segno;
> > 

[toc] | [prev] | [next] | [standalone]

#1557362 — Re: [f2fs-dev] [PATCH 01/10] f2fs: reassign new segment for mode=lfs

On 2017/1/5 6:48, Jaegeuk Kim wrote:
> On 01/04, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2016/12/31 2:51, Jaegeuk Kim wrote:
>>> Otherwise we can remain wrong curseg->next_blkoff, resulting in fsck failure.
>>
>> Could you explain more about this case?
> 
> I remember that I hit an fsck failure when I was testing f2fs with an smr drive.
> I didn't dig into the error, but the fact is that our roll-forward recovery
> doesn't update current segment information at every time, but allocate a new
> section at the end of the work like below.
> I just enabled it for the LFS mode in order to avoid that failure.

Alright, thanks for the explanation.

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>> ---
>>>  fs/f2fs/segment.c | 3 ---
>>>  1 file changed, 3 deletions(-)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index be9e4d244d75..4e5ffe1d97e4 100644
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -1428,9 +1428,6 @@ void allocate_new_segments(struct f2fs_sb_info *sbi)
>>>  	unsigned int old_segno;
>>>  	int i;
>>>  
>>> -	if (test_opt(sbi, LFS))
>>> -		return;
>>> -
>>>  	for (i = CURSEG_HOT_DATA; i <= CURSEG_COLD_DATA; i++) {
>>>  		curseg = CURSEG_I(sbi, i);
>>>  		old_segno = curseg->segno;
>>>
> 
> .
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web