subject:"Re\: \[f2fs\-dev\] \[PATCH v4 4\/4\] f2fs\: stop checkpoint when get a out\-of\-bounds segment"

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

2024-02-22 Thread Chao Yu


On 2024/2/22 15:15, Zhiguo Niu wrote:



On Thu, Feb 22, 2024 at 2:12 PM Chao Yu mailto:c...@kernel.org>> wrote:

On 2024/2/22 13:48, Zhiguo Niu wrote:
 > Dear Chao,
 >
 > On Thu, Feb 22, 2024 at 11:51 AM Chao Yu mailto:c...@kernel.org> 
>> wrote:
 >
 >     On 2024/2/22 10:15, Zhiguo Niu wrote:
 >      >
 >      >
 >      > On Thu, Feb 22, 2024 at 9:37 AM Chao Yu mailto:c...@kernel.org> 
>  
      >
 >      >     On 2024/2/20 14:11, Zhiguo Niu wrote:
 >      >      > There is low probability that an out-of-bounds segment 
will be got
 >      >      > on a small-capacity device. In order to prevent subsequent 
write requests
 >      >      > allocating block address from this invalid segment, which 
may cause
 >      >      > unexpected issue, stop checkpoint should be performed.
 >      >      >
 >      >      > Also introduce a new stop cp reason: 
STOP_CP_REASON_NO_SEGMENT.
 >      >      >
 >      >      > Signed-off-by: Zhiguo Niu mailto:zhiguo@unisoc.com> 
>        >      > ---
 >      >      > changes of v4: use more suitable MACRO name according to 
Chao's suggestions
 >      >      > changes of v3: correct MACRO spelling and update based on 
the lastes code
 >      >      > ---
 >      >      > ---
 >      >      >   fs/f2fs/segment.c       | 7 ++-
 >      >      >   include/linux/f2fs_fs.h | 1 +
 >      >      >   2 files changed, 7 insertions(+), 1 deletion(-)
 >      >      >
 >      >      > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
 >      >      > index c25aaec..e42e34c 100644
 >      >      > --- a/fs/f2fs/segment.c
 >      >      > +++ b/fs/f2fs/segment.c
 >      >      > @@ -2665,7 +2665,12 @@ static void get_new_segment(struct 
f2fs_sb_info *sbi,
 >      >      >       if (secno >= MAIN_SECS(sbi)) {
 >      >      >               secno = 
find_first_zero_bit(free_i->free_secmap,
 >      >      >                                                       
MAIN_SECS(sbi));
 >      >      > -             f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
 >      >      > +             if (secno >= MAIN_SECS(sbi)) {
 >      >      > +                     f2fs_stop_checkpoint(sbi, false,
 >      >      > +                             STOP_CP_REASON_NO_SEGMENT);
 >      >
 >      >     We should relocate stop_checkpoint(sbi, false, 
STOP_CP_REASON_NO_SEGMENT) outside
 >      >     segmap_lock spinlock, due to it may sleep in 
f2fs_flush_merged_writes().
 >      >
 >      > Indeed it is. How about the following fix?
 >      > @@ -2665,11 +2665,7 @@ static void get_new_segment(struct 
f2fs_sb_info *sbi,
 >      >          if (secno >= MAIN_SECS(sbi)) {
 >      >                  secno = find_first_zero_bit(free_i->free_secmap,
 >      >                                                          
MAIN_SECS(sbi));
 >      > -               if (secno >= MAIN_SECS(sbi)) {
 >      > -                       f2fs_stop_checkpoint(sbi, false,
 >      > -                               STOP_CP_REASON_NO_SEGMENT);
 >      > -                       f2fs_bug_on(sbi, 1);
 >      > -               }
 >      > +               f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
 >      >          }
 >      >          segno = GET_SEG_FROM_SEC(sbi, secno);
 >      >          zoneno = GET_ZONE_FROM_SEC(sbi, secno);
 >      > @@ -2700,6 +2696,8 @@ static void get_new_segment(struct 
f2fs_sb_info *sbi,
 >      >          __set_inuse(sbi, segno);
 >      >          *newseg = segno;
 >      >          spin_unlock(_i->segmap_lock);
 >      > +       if (secno >= MAIN_SECS(sbi))
 >      > +               f2fs_stop_checkpoint(sbi, false, 
STOP_CP_REASON_NO_SEGMENT);
 >
 >     How about this?
 >
 >     ---
 >        fs/f2fs/segment.c | 12 +---
 >        1 file changed, 9 insertions(+), 3 deletions(-)
 >
 >     diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
 >     index d0209ea77dd2..8edc42071e6f 100644
 >     --- a/fs/f2fs/segment.c
 >     +++ b/fs/f2fs/segment.c
 >     @@ -2646,6 +2646,7 @@ static void get_new_segment(struct 
f2fs_sb_info *sbi,
 >              unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
 >              bool init = true;
 >              int i;
 >     +       int ret = 0;
 >
 >              spin_lock(_i->segmap_lock);
 >
 >     @@ -2671,9 +2672,8 @@ static void

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

2024-02-21 Thread Chao Yu


On 2024/2/22 13:48, Zhiguo Niu wrote:

Dear Chao,

On Thu, Feb 22, 2024 at 11:51 AM Chao Yu mailto:c...@kernel.org>> wrote:

On 2024/2/22 10:15, Zhiguo Niu wrote:
 >
 >
 > On Thu, Feb 22, 2024 at 9:37 AM Chao Yu mailto:c...@kernel.org> 
>> wrote:
 >
 >     On 2024/2/20 14:11, Zhiguo Niu wrote:
 >      > There is low probability that an out-of-bounds segment will be got
 >      > on a small-capacity device. In order to prevent subsequent write 
requests
 >      > allocating block address from this invalid segment, which may 
cause
 >      > unexpected issue, stop checkpoint should be performed.
 >      >
 >      > Also introduce a new stop cp reason: STOP_CP_REASON_NO_SEGMENT.
 >      >
 >      > Signed-off-by: Zhiguo Niu mailto:zhiguo@unisoc.com> 
>>
 >      > ---
 >      > changes of v4: use more suitable MACRO name according to Chao's 
suggestions
 >      > changes of v3: correct MACRO spelling and update based on the 
lastes code
 >      > ---
 >      > ---
 >      >   fs/f2fs/segment.c       | 7 ++-
 >      >   include/linux/f2fs_fs.h | 1 +
 >      >   2 files changed, 7 insertions(+), 1 deletion(-)
 >      >
 >      > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
 >      > index c25aaec..e42e34c 100644
 >      > --- a/fs/f2fs/segment.c
 >      > +++ b/fs/f2fs/segment.c
 >      > @@ -2665,7 +2665,12 @@ static void get_new_segment(struct 
f2fs_sb_info *sbi,
 >      >       if (secno >= MAIN_SECS(sbi)) {
 >      >               secno = find_first_zero_bit(free_i->free_secmap,
 >      >                                                       
MAIN_SECS(sbi));
 >      > -             f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
 >      > +             if (secno >= MAIN_SECS(sbi)) {
 >      > +                     f2fs_stop_checkpoint(sbi, false,
 >      > +                             STOP_CP_REASON_NO_SEGMENT);
 >
 >     We should relocate stop_checkpoint(sbi, false, 
STOP_CP_REASON_NO_SEGMENT) outside
 >     segmap_lock spinlock, due to it may sleep in 
f2fs_flush_merged_writes().
 >
 > Indeed it is. How about the following fix?
 > @@ -2665,11 +2665,7 @@ static void get_new_segment(struct f2fs_sb_info 
*sbi,
 >          if (secno >= MAIN_SECS(sbi)) {
 >                  secno = find_first_zero_bit(free_i->free_secmap,
 >                                                          MAIN_SECS(sbi));
 > -               if (secno >= MAIN_SECS(sbi)) {
 > -                       f2fs_stop_checkpoint(sbi, false,
 > -                               STOP_CP_REASON_NO_SEGMENT);
 > -                       f2fs_bug_on(sbi, 1);
 > -               }
 > +               f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
 >          }
 >          segno = GET_SEG_FROM_SEC(sbi, secno);
 >          zoneno = GET_ZONE_FROM_SEC(sbi, secno);
 > @@ -2700,6 +2696,8 @@ static void get_new_segment(struct f2fs_sb_info 
*sbi,
 >          __set_inuse(sbi, segno);
 >          *newseg = segno;
 >          spin_unlock(_i->segmap_lock);
 > +       if (secno >= MAIN_SECS(sbi))
 > +               f2fs_stop_checkpoint(sbi, false, 
STOP_CP_REASON_NO_SEGMENT);

How about this?

---
   fs/f2fs/segment.c | 12 +---
   1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index d0209ea77dd2..8edc42071e6f 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2646,6 +2646,7 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
         unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
         bool init = true;
         int i;
+       int ret = 0;

         spin_lock(_i->segmap_lock);

@@ -2671,9 +2672,8 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
                 secno = find_first_zero_bit(free_i->free_secmap,
                                                         MAIN_SECS(sbi));
                 if (secno >= MAIN_SECS(sbi)) {
-                       f2fs_stop_checkpoint(sbi, false,
-                               STOP_CP_REASON_NO_SEGMENT);
-                       f2fs_bug_on(sbi, 1);
+                       ret = -ENOSPC;
+                       goto out_unlock; 


                 }
         }
         segno = GET_SEG_FROM_SEC(sbi, secno);
@@ -2704,7 +2704,13 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
         f2fs_bug_on(sbi, test_bit(segno, free_i->free_segmap));
         __set_inuse(sbi, segno);
         *newseg = segno;
+out_unlock:
         spin_unlock(_i->segmap_lock);
+
+       if (ret) {
+               f2fs_stop_checkpoint(sbi, false, STOP_CP_REASON_NO_SEGMENT);

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

2024-02-21 Thread Chao Yu


On 2024/2/22 10:15, Zhiguo Niu wrote:



On Thu, Feb 22, 2024 at 9:37 AM Chao Yu mailto:c...@kernel.org>> wrote:

On 2024/2/20 14:11, Zhiguo Niu wrote:
 > There is low probability that an out-of-bounds segment will be got
 > on a small-capacity device. In order to prevent subsequent write requests
 > allocating block address from this invalid segment, which may cause
 > unexpected issue, stop checkpoint should be performed.
 >
 > Also introduce a new stop cp reason: STOP_CP_REASON_NO_SEGMENT.
 >
 > Signed-off-by: Zhiguo Niu mailto:zhiguo@unisoc.com>>
 > ---
 > changes of v4: use more suitable MACRO name according to Chao's 
suggestions
 > changes of v3: correct MACRO spelling and update based on the lastes code
 > ---
 > ---
 >   fs/f2fs/segment.c       | 7 ++-
 >   include/linux/f2fs_fs.h | 1 +
 >   2 files changed, 7 insertions(+), 1 deletion(-)
 >
 > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
 > index c25aaec..e42e34c 100644
 > --- a/fs/f2fs/segment.c
 > +++ b/fs/f2fs/segment.c
 > @@ -2665,7 +2665,12 @@ static void get_new_segment(struct f2fs_sb_info 
*sbi,
 >       if (secno >= MAIN_SECS(sbi)) {
 >               secno = find_first_zero_bit(free_i->free_secmap,
 >                                                       MAIN_SECS(sbi));
 > -             f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
 > +             if (secno >= MAIN_SECS(sbi)) {
 > +                     f2fs_stop_checkpoint(sbi, false,
 > +                             STOP_CP_REASON_NO_SEGMENT);

We should relocate stop_checkpoint(sbi, false, STOP_CP_REASON_NO_SEGMENT) 
outside
segmap_lock spinlock, due to it may sleep in f2fs_flush_merged_writes().

Indeed it is. How about the following fix?
@@ -2665,11 +2665,7 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
         if (secno >= MAIN_SECS(sbi)) {
                 secno = find_first_zero_bit(free_i->free_secmap,
                                                         MAIN_SECS(sbi));
-               if (secno >= MAIN_SECS(sbi)) {
-                       f2fs_stop_checkpoint(sbi, false,
-                               STOP_CP_REASON_NO_SEGMENT);
-                       f2fs_bug_on(sbi, 1);
-               }
+               f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
         }
         segno = GET_SEG_FROM_SEC(sbi, secno);
         zoneno = GET_ZONE_FROM_SEC(sbi, secno);
@@ -2700,6 +2696,8 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
         __set_inuse(sbi, segno);
         *newseg = segno;
         spin_unlock(_i->segmap_lock);
+       if (secno >= MAIN_SECS(sbi))
+               f2fs_stop_checkpoint(sbi, false, STOP_CP_REASON_NO_SEGMENT);


How about this?

---
 fs/f2fs/segment.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index d0209ea77dd2..8edc42071e6f 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2646,6 +2646,7 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
unsigned int old_zoneno = GET_ZONE_FROM_SEG(sbi, *newseg);
bool init = true;
int i;
+   int ret = 0;

spin_lock(_i->segmap_lock);

@@ -2671,9 +2672,8 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
secno = find_first_zero_bit(free_i->free_secmap,
MAIN_SECS(sbi));
if (secno >= MAIN_SECS(sbi)) {
-   f2fs_stop_checkpoint(sbi, false,
-   STOP_CP_REASON_NO_SEGMENT);
-   f2fs_bug_on(sbi, 1);
+   ret = -ENOSPC;
+   goto out_unlock;
}
}
segno = GET_SEG_FROM_SEC(sbi, secno);
@@ -2704,7 +2704,13 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
f2fs_bug_on(sbi, test_bit(segno, free_i->free_segmap));
__set_inuse(sbi, segno);
*newseg = segno;
+out_unlock:
spin_unlock(_i->segmap_lock);
+
+   if (ret) {
+   f2fs_stop_checkpoint(sbi, false, STOP_CP_REASON_NO_SEGMENT);
+   f2fs_bug_on(sbi, 1);
+   }
 }

 static void reset_curseg(struct f2fs_sb_info *sbi, int type, int modified)
--
2.40.1



Thanks,

 > +                     f2fs_bug_on(sbi, 1);
 > +             }
 > +
 >       }
 >       segno = GET_SEG_FROM_SEC(sbi, secno);
 >       zoneno = GET_ZONE_FROM_SEC(sbi, secno);
 > diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
 > index 9b69c50..755e9a4 100644
 > --- a/include/linux/f2fs_fs.h
 > +++ b/include/linux/f2fs_fs.h
 > @@ -75,6 +75,7 @@ enum stop_cp_reason {
 >       STOP_CP_REASON_CORRUPTED_SUMMARY,
 >       STOP_CP_REASON_UPDATE_INODE,
 >       STOP_CP_REASON_FLUSH_FAIL,
 > +     STOP_CP_REASON_NO_SEGMENT,
 >       STOP_CP_REASON_MAX,
 >   };
 >

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

2024-02-21 Thread Chao Yu


On 2024/2/20 14:11, Zhiguo Niu wrote:

There is low probability that an out-of-bounds segment will be got
on a small-capacity device. In order to prevent subsequent write requests
allocating block address from this invalid segment, which may cause
unexpected issue, stop checkpoint should be performed.

Also introduce a new stop cp reason: STOP_CP_REASON_NO_SEGMENT.

Signed-off-by: Zhiguo Niu 
---
changes of v4: use more suitable MACRO name according to Chao's suggestions
changes of v3: correct MACRO spelling and update based on the lastes code
---
---
  fs/f2fs/segment.c   | 7 ++-
  include/linux/f2fs_fs.h | 1 +
  2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index c25aaec..e42e34c 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2665,7 +2665,12 @@ static void get_new_segment(struct f2fs_sb_info *sbi,
if (secno >= MAIN_SECS(sbi)) {
secno = find_first_zero_bit(free_i->free_secmap,
MAIN_SECS(sbi));
-   f2fs_bug_on(sbi, secno >= MAIN_SECS(sbi));
+   if (secno >= MAIN_SECS(sbi)) {
+   f2fs_stop_checkpoint(sbi, false,
+   STOP_CP_REASON_NO_SEGMENT);


We should relocate stop_checkpoint(sbi, false, STOP_CP_REASON_NO_SEGMENT) 
outside
segmap_lock spinlock, due to it may sleep in f2fs_flush_merged_writes().

Thanks,


+   f2fs_bug_on(sbi, 1);
+   }
+
}
segno = GET_SEG_FROM_SEC(sbi, secno);
zoneno = GET_ZONE_FROM_SEC(sbi, secno);
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index 9b69c50..755e9a4 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -75,6 +75,7 @@ enum stop_cp_reason {
STOP_CP_REASON_CORRUPTED_SUMMARY,
STOP_CP_REASON_UPDATE_INODE,
STOP_CP_REASON_FLUSH_FAIL,
+   STOP_CP_REASON_NO_SEGMENT,
STOP_CP_REASON_MAX,
  };
  



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

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

2024-02-19 Thread Chao Yu


On 2024/2/20 14:11, Zhiguo Niu wrote:

There is low probability that an out-of-bounds segment will be got
on a small-capacity device. In order to prevent subsequent write requests
allocating block address from this invalid segment, which may cause
unexpected issue, stop checkpoint should be performed.

Also introduce a new stop cp reason: STOP_CP_REASON_NO_SEGMENT.


Can you please adjust f2fs-tools as well?



Signed-off-by: Zhiguo Niu 


Reviewed-by: Chao Yu 

Thanks,


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

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

Re: [f2fs-dev] [PATCH v4 4/4] f2fs: stop checkpoint when get a out-of-bounds segment

5 matches

Site Navigation

Mail list logo

Footer information