[f2fs-dev] [PATCH v1] f2fs: add f2fs_gc exception handle in f2fs_ioc_gc_range

[f2fs-dev] [PATCH v1] f2fs: add f2fs_gc exception handle in f2fs_ioc_gc_range

[Qilong Zhang]

When f2fs_ioc_gc_range performs multiple segments gc ops, the return
value of f2fs_ioc_gc_range is determined by the last segment gc ops.
If its ops failed, the f2fs_ioc_gc_range will be considered to be failed
despite some of previous segments gc ops succeeded. Therefore, so we
fix: only all of segments gc ops are finished will return success. In
particular, 1).if target has no valid blopcks, it will go on. 2).if
target sectoion has valid blocks, but it is current segment, we will
reminder the caller.

Signed-off-by: Qilong Zhang <zhangqilong3@huawei.com>
---
v1:
- handle the -ENODATA exceptions of f2fs_gc.
 fs/f2fs/file.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 3268f8dd59bb..64415bd72ac4 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2527,6 +2527,18 @@ static int f2fs_ioc_gc_range(struct file *filp, unsigned long arg)
 	}
 
 	ret = f2fs_gc(sbi, range.sync, true, GET_SEGNO(sbi, range.start));
+	if (ret) {
+		if (ret == -ENODATA) {
+			if (IS_CURSEG(sbi, range.start)) {
+				/* it is opened, could try later*/
+				ret = -EBUSY;
+				goto out;
+			}
+		} else {
+			ret = -EAGAIN;
+			goto out;
+		}
+	}
 	range.start += BLKS_PER_SEC(sbi);
 	if (range.start <= end)
 		goto do_more;
-- 
2.17.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v1] f2fs: add f2fs_gc exception handle in f2fs_ioc_gc_range

[Chao Yu]

On 2020/6/23 22:34, Qilong Zhang wrote:
> When f2fs_ioc_gc_range performs multiple segments gc ops, the return
> value of f2fs_ioc_gc_range is determined by the last segment gc ops.
> If its ops failed, the f2fs_ioc_gc_range will be considered to be failed
> despite some of previous segments gc ops succeeded. Therefore, so we
> fix: only all of segments gc ops are finished will return success. In
> particular, 1).if target has no valid blopcks, it will go on. 2).if
> target sectoion has valid blocks, but it is current segment, we will
> reminder the caller.
> 
> Signed-off-by: Qilong Zhang <zhangqilong3@huawei.com>
> ---
> v1:
> - handle the -ENODATA exceptions of f2fs_gc.

Should be v2?

I mean this:

---
 fs/f2fs/gc.c      | 24 ++++++++++++++++--------
 fs/f2fs/segment.c |  4 ++--
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 5e1368ce2389..e017c79a11d0 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -330,6 +330,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
 	unsigned int secno, last_victim;
 	unsigned int last_segment;
 	unsigned int nsearched = 0;
+	int ret;

 	mutex_lock(&dirty_i->seglist_lock);
 	last_segment = MAIN_SECS(sbi) * sbi->segs_per_sec;
@@ -341,12 +342,20 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
 	p.min_cost = get_max_cost(sbi, &p);

 	if (*result != NULL_SEGNO) {
-		if (get_valid_blocks(sbi, *result, false) &&
-			!sec_usage_check(sbi, GET_SEC_FROM_SEG(sbi, *result)))
-			p.min_segno = *result;
+		if (!get_valid_blocks(sbi, *result, true)) {
+			ret = 0;
+			goto out;
+		}
+		if (sec_usage_check(sbi, GET_SEC_FROM_SEG(sbi, *result))) {
+			ret = -EAGAIN;
+			goto out;
+		}
+		p.min_segno = *result;
 		goto out;
 	}

+	ret = -ENODATA;
+
 	if (p.max_search == 0)
 		goto out;

@@ -447,7 +456,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
 			else
 				set_bit(secno, dirty_i->victim_secmap);
 		}
-
+		ret = 0;
 	}
 out:
 	if (p.min_segno != NULL_SEGNO)
@@ -456,7 +465,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi,
 				prefree_segments(sbi), free_segments(sbi));
 	mutex_unlock(&dirty_i->seglist_lock);

-	return (p.min_segno == NULL_SEGNO) ? 0 : 1;
+	return ret;
 }

 static const struct victim_selection default_v_ops = {
@@ -1340,10 +1349,9 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
 		ret = -EINVAL;
 		goto stop;
 	}
-	if (!__get_victim(sbi, &segno, gc_type)) {
-		ret = -ENODATA;
+	ret = __get_victim(sbi, &segno, gc_type);
+	if (ret)
 		goto stop;
-	}

 	seg_freed = do_garbage_collect(sbi, segno, &gc_list, gc_type);
 	if (gc_type == FG_GC && seg_freed == sbi->segs_per_sec)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 735c598c25ea..5aa4158a1440 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2631,7 +2631,7 @@ static int get_ssr_segment(struct f2fs_sb_info *sbi, int type)
 	bool reversed = false;

 	/* f2fs_need_SSR() already forces to do this */
-	if (v_ops->get_victim(sbi, &segno, BG_GC, type, SSR)) {
+	if (!v_ops->get_victim(sbi, &segno, BG_GC, type, SSR)) {
 		curseg->next_segno = segno;
 		return 1;
 	}
@@ -2658,7 +2658,7 @@ static int get_ssr_segment(struct f2fs_sb_info *sbi, int type)
 	for (; cnt-- > 0; reversed ? i-- : i++) {
 		if (i == type)
 			continue;
-		if (v_ops->get_victim(sbi, &segno, BG_GC, i, SSR)) {
+		if (!v_ops->get_victim(sbi, &segno, BG_GC, i, SSR)) {
 			curseg->next_segno = segno;
 			return 1;
 		}
-- 
2.26.2



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel