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 <hsiang...@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


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to