linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [f2fs-dev][PATCH RFC] f2fs: refactor get_new_segment
@ 2017-12-16  8:32 Gao Xiang
  2017-12-19 22:56 ` Jaegeuk Kim
  0 siblings, 1 reply; 2+ messages in thread
From: Gao Xiang @ 2017-12-16  8:32 UTC (permalink / raw)
  To: jaegeuk, chao; +Cc: linux-f2fs-devel, linux-fsdevel, Gao Xiang

get_new_segment is too unclear to understand how it works.
This patch refactor it in a straight-forward way and
I think it is equal to the original one.

This patch also fixes two issues in the original get_new_segment:
1) left_start could be overflowed when hint == 0 at first:
	...
		} else {
			go_left = 1;
*			left_start = hint - 1;
		}
	...
*	while (test_bit(left_start, free_i->free_secmap)) {
2) It will do find_next_zero_bit again when go_left == true and ALLOC_LEFT:
	...
find_other_zone:
*	secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), hint);
	if (secno >= MAIN_SECS(sbi)) {
		if (dir == ALLOC_RIGHT) {
			...
		} else {
			go_left = 1;
			...
		}
	}
	if (go_left == 0)
		goto skip_left;
	...
	if (i < NR_CURSEG_TYPE) {
		/* zone is in user, try another */
*		if (go_left)
*			hint = zoneno * sbi->secs_per_zone - 1;
		...
		init = false;
		goto find_other_zone;
	}

Signed-off-by: Gao Xiang <hsiangkao@aol.com>
---
 fs/f2fs/segment.c | 137 ++++++++++++++++++++++++++++++++----------------------
 1 file changed, 81 insertions(+), 56 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index c117e09..eea9d3f 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2047,82 +2047,107 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
 			unsigned int *newseg, bool new_sec, int dir)
 {
 	struct free_segmap_info *free_i = FREE_I(sbi);
-	unsigned int segno, secno, zoneno;
+	unsigned int segno = *newseg, zoneno;
+	unsigned int secno = GET_SEC_FROM_SEG(sbi, segno);
 	unsigned int total_zones = MAIN_SECS(sbi) / sbi->secs_per_zone;
-	unsigned int hint = GET_SEC_FROM_SEG(sbi, *newseg);
-	unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
-	unsigned int left_start = hint;
-	bool init = true;
-	int go_left = 0;
+	unsigned int old_zoneno = GET_ZONE_FROM_SEC(sbi, secno);
+	bool may_again = true, actually_go_left = false;
 	int i;
 
 	spin_lock(&free_i->segmap_lock);
 
-	if (!new_sec && ((*newseg + 1) % sbi->segs_per_sec)) {
+	/* first, attempt to find a segment in the current section */
+	if (!new_sec && ((segno + 1) % sbi->segs_per_sec)) {
+		unsigned end_segno = GET_SEG_FROM_SEC(sbi, secno + 1);
+
 		segno = find_next_zero_bit(free_i->free_segmap,
-			GET_SEG_FROM_SEC(sbi, hint + 1), *newseg + 1);
-		if (segno < GET_SEG_FROM_SEC(sbi, hint + 1))
-			goto got_it;
+			end_segno, segno + 1);
+
+		if (segno < end_segno)
+			goto out;
 	}
-find_other_zone:
-	secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), hint);
-	if (secno >= MAIN_SECS(sbi)) {
-		if (dir == ALLOC_RIGHT) {
-			secno = find_next_zero_bit(free_i->free_secmap,
-							MAIN_SECS(sbi), 0);
-			f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
-		} else {
-			go_left = 1;
-			left_start = hint - 1;
+
+try_another_section:
+	if (likely(!actually_go_left)) {
+		/*
+		 * since ALLOC_LEFT takes much effort,
+		 * prefer to ALLOC_RIGHT first
+		 */
+		unsigned int new_secno = find_next_zero_bit(
+			free_i->free_secmap, MAIN_SECS(sbi), secno);
+
+		if (new_secno < MAIN_SECS(sbi)) {
+			secno = new_secno;
+			goto check_another_zone;
 		}
 	}
-	if (go_left == 0)
-		goto skip_left;
 
-	while (test_bit(left_start, free_i->free_secmap)) {
-		if (left_start > 0) {
-			left_start--;
-			continue;
-		}
-		left_start = find_next_zero_bit(free_i->free_secmap,
-							MAIN_SECS(sbi), 0);
-		f2fs_bug_on(sbi, left_start >= MAIN_SECS(sbi));
-		break;
+	if (dir == ALLOC_LEFT) {
+		/* ALLOC_LEFT, no free sections on the right side */
+		actually_go_left = true;
+
+		while(secno)
+			if (!test_bit(--secno, free_i->free_secmap))
+				goto check_another_zone;
 	}
-	secno = left_start;
-skip_left:
-	segno = GET_SEG_FROM_SEC(sbi, secno);
-	zoneno = GET_ZONE_FROM_SEC(sbi, secno);
 
+	/*
+	 * when ALLOC_RIGHT and secno >= end_endno
+	 *   or  ALLOC_LEFT and secno == 0, we need do the last attempt
+	 * since the function should be returned with success
+	 */
+	secno = find_next_zero_bit(free_i->free_secmap, MAIN_SECS(sbi), 0);
+	f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
+
+check_another_zone:
 	/* give up on finding another zone */
-	if (!init)
-		goto got_it;
-	if (sbi->secs_per_zone == 1)
+	if (!may_again || sbi->secs_per_zone == 1)
 		goto got_it;
+
+	zoneno = GET_ZONE_FROM_SEC(sbi, secno);
 	if (zoneno == old_zoneno)
 		goto got_it;
-	if (dir == ALLOC_LEFT) {
-		if (!go_left && zoneno + 1 >= total_zones)
-			goto got_it;
-		if (go_left && zoneno == 0)
+
+	/*
+	 * since the following case will be likely to frequently happen,
+	 * leave it here rather than put together in the following loop
+	 */
+	if (dir == ALLOC_LEFT)
+		/*
+		 * I'm really confused why not select
+		 * zoneno * sbi->secs_per_zone - 1 in this case.
+		 * Okay, leave it as the origin
+		 */
+		if (!actually_go_left && zoneno + 1 >= total_zones)
 			goto got_it;
-	}
+
 	for (i = 0; i < NR_CURSEG_TYPE; i++)
-		if (CURSEG_I(sbi, i)->zone == zoneno)
-			break;
+		/* the zone is in use, try another */
+		if (CURSEG_I(sbi, i)->zone == zoneno) {
+			/* try the neighbor (left or right) zone */
+			if (actually_go_left) {
+				/*
+				 * since ALLOC_LEFT and no way to get free on
+				 * the right side, it's safe to try the left
+				 * zone and won't find the same section again.
+				 */
+				if (zoneno == 0)
+					break;
+				secno = zoneno * sbi->secs_per_zone - 1;
+			} else if (zoneno + 1 >= total_zones) {
+				/* no way dir == ALLOC_LEFT at this time */
+				secno = 0;
+			} else
+				secno = (zoneno + 1) * sbi->secs_per_zone;
+
+			/* try another zone once at most */
+			may_again = false;
+			goto try_another_section;
+		}
 
-	if (i < NR_CURSEG_TYPE) {
-		/* zone is in user, try another */
-		if (go_left)
-			hint = zoneno * sbi->secs_per_zone - 1;
-		else if (zoneno + 1 >= total_zones)
-			hint = 0;
-		else
-			hint = (zoneno + 1) * sbi->secs_per_zone;
-		init = false;
-		goto find_other_zone;
-	}
 got_it:
+	segno = GET_SEG_FROM_SEC(sbi, secno);
+out:
 	/* set it as dirty segment in free segmap */
 	f2fs_bug_on(sbi, test_bit(segno, free_i->free_segmap));
 	__set_inuse(sbi, segno);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-12-19 22:56 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-16  8:32 [f2fs-dev][PATCH RFC] f2fs: refactor get_new_segment Gao Xiang
2017-12-19 22:56 ` Jaegeuk Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).