diff mbox series

f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize

Message ID 1709292976-13118-1-git-send-email-zhiguo.niu@unisoc.com
State New
Headers
Series f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize |

Commit Message

Zhiguo Niu March 1, 2024, 11:36 a.m. UTC 
  new_curseg may return error if get_new_segment fail, so its result
should be check in its caller f2fs_allocate_segment_for_resize,
alos pass this results to free_segment_range.

Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
---
 fs/f2fs/f2fs.h    | 2 +-
 fs/f2fs/gc.c      | 7 +++++--
 fs/f2fs/segment.c | 9 +++++++--
 3 files changed, 13 insertions(+), 5 deletions(-)

Comments

Chao Yu March 4, 2024, 3:19 a.m. UTC | #1 
On 2024/3/1 19:36, Zhiguo Niu wrote:
> new_curseg may return error if get_new_segment fail, so its result
> should be check in its caller f2fs_allocate_segment_for_resize,
> alos pass this results to free_segment_range.

Zhiguo,

What about handling all error paths of new_curseg() and change_curseg()
in one patch?

Thanks,

> 
> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
> ---
>   fs/f2fs/f2fs.h    | 2 +-
>   fs/f2fs/gc.c      | 7 +++++--
>   fs/f2fs/segment.c | 9 +++++++--
>   3 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 4331012..39dda7d 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
>   void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
>   void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
>   void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>   					unsigned int start, unsigned int end);
>   int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
>   int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index c60b747..7a458fa 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -2037,8 +2037,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
>   	mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
>   
>   	/* Move out cursegs from the target range */
> -	for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
> -		f2fs_allocate_segment_for_resize(sbi, type, start, end);
> +	for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
> +		err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
> +		if (err)
> +			goto out;
> +	}
>   
>   	/* do GC to move out valid blocks in the range */
>   	err = f2fs_gc_range(sbi, start, end, dry_run, 0);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 1bb3019..2a07b9d 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -3071,11 +3071,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
>   	return false;
>   }
>   
> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>   					unsigned int start, unsigned int end)
>   {
>   	struct curseg_info *curseg = CURSEG_I(sbi, type);
>   	unsigned int segno;
> +	int err = 0;
>   
>   	f2fs_down_read(&SM_I(sbi)->curseg_lock);
>   	mutex_lock(&curseg->curseg_mutex);
> @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>   		change_curseg(sbi, type);
>   	else
>   		new_curseg(sbi, type, true);
> -
> +	if (curseg->segno == NULL_SEGNO) {
> +		err = -ENOSPC;
> +		goto unlock;
> +	}
>   	stat_inc_seg_type(sbi, curseg);
>   
>   	locate_dirty_segment(sbi, segno);
> @@ -3102,6 +3106,7 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>   
>   	mutex_unlock(&curseg->curseg_mutex);
>   	f2fs_up_read(&SM_I(sbi)->curseg_lock);
> +	return err;
>   }
>   
>   static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
Zhiguo Niu March 4, 2024, 3:33 a.m. UTC | #2 
On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote:
>
> On 2024/3/1 19:36, Zhiguo Niu wrote:
> > new_curseg may return error if get_new_segment fail, so its result
> > should be check in its caller f2fs_allocate_segment_for_resize,
> > alos pass this results to free_segment_range.
>
> Zhiguo,
>
> What about handling all error paths of new_curseg() and change_curseg()
> in one patch?
Dear Chao,

Do you mean to merge it with the previous patch “f2fs: fix to check
return value of f2fs_gc_range”?
Because in addition to new_curseg/change_curseg error handling, there
are some other changes in the previous patch.
besides, I searched for new related codes, and there should be the
only place left without error handling about new_curseg/
change_curseg .

thanks!
>
> Thanks,
>
> >
> > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
> > ---
> >   fs/f2fs/f2fs.h    | 2 +-
> >   fs/f2fs/gc.c      | 7 +++++--
> >   fs/f2fs/segment.c | 9 +++++++--
> >   3 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 4331012..39dda7d 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
> >   void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
> >   void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
> >   void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
> > -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> > +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >                                       unsigned int start, unsigned int end);
> >   int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
> >   int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index c60b747..7a458fa 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -2037,8 +2037,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
> >       mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
> >
> >       /* Move out cursegs from the target range */
> > -     for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
> > -             f2fs_allocate_segment_for_resize(sbi, type, start, end);
> > +     for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
> > +             err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
> > +             if (err)
> > +                     goto out;
> > +     }
> >
> >       /* do GC to move out valid blocks in the range */
> >       err = f2fs_gc_range(sbi, start, end, dry_run, 0);
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 1bb3019..2a07b9d 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -3071,11 +3071,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
> >       return false;
> >   }
> >
> > -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> > +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >                                       unsigned int start, unsigned int end)
> >   {
> >       struct curseg_info *curseg = CURSEG_I(sbi, type);
> >       unsigned int segno;
> > +     int err = 0;
> >
> >       f2fs_down_read(&SM_I(sbi)->curseg_lock);
> >       mutex_lock(&curseg->curseg_mutex);
> > @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >               change_curseg(sbi, type);
> >       else
> >               new_curseg(sbi, type, true);
> > -
> > +     if (curseg->segno == NULL_SEGNO) {
> > +             err = -ENOSPC;
> > +             goto unlock;
> > +     }
> >       stat_inc_seg_type(sbi, curseg);
> >
> >       locate_dirty_segment(sbi, segno);
> > @@ -3102,6 +3106,7 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >
> >       mutex_unlock(&curseg->curseg_mutex);
> >       f2fs_up_read(&SM_I(sbi)->curseg_lock);
> > +     return err;
> >   }
> >
> >   static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
Chao Yu March 4, 2024, 7:17 a.m. UTC | #3 
On 2024/3/4 11:33, Zhiguo Niu wrote:
> On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote:
>>
>> On 2024/3/1 19:36, Zhiguo Niu wrote:
>>> new_curseg may return error if get_new_segment fail, so its result
>>> should be check in its caller f2fs_allocate_segment_for_resize,
>>> alos pass this results to free_segment_range.
>>
>> Zhiguo,
>>
>> What about handling all error paths of new_curseg() and change_curseg()
>> in one patch?
> Dear Chao,
> 
> Do you mean to merge it with the previous patch “f2fs: fix to check
> return value of f2fs_gc_range”?
> Because in addition to new_curseg/change_curseg error handling, there
> are some other changes in the previous patch.
> besides, I searched for new related codes, and there should be the
> only place left without error handling about new_curseg/
> change_curseg .

Zhiguo, I meant something like this?

Subject: [PATCH] f2fs: fix to handle error paths of {new,change}_curseg()

---
  fs/f2fs/f2fs.h    |  4 +--
  fs/f2fs/gc.c      |  7 +++--
  fs/f2fs/segment.c | 67 +++++++++++++++++++++++++++++++----------------
  fs/f2fs/super.c   |  4 ++-
  4 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 80789255bf68..03927f1b2ea1 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3702,10 +3702,10 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable);
  void f2fs_release_discard_addrs(struct f2fs_sb_info *sbi);
  int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra);
  bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno);
-void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
+int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
  void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
  void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
-void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
+int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
  					unsigned int start, unsigned int end);
  int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
  int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index f8314765246a..854ad0a3f6ea 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -2033,8 +2033,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
  	mutex_unlock(&DIRTY_I(sbi)->seglist_lock);

  	/* Move out cursegs from the target range */
-	for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
-		f2fs_allocate_segment_for_resize(sbi, type, start, end);
+	for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
+		err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
+		if (err)
+			goto out;
+	}

  	/* do GC to move out valid blocks in the range */
  	err = f2fs_gc_range(sbi, start, end, dry_run, 0);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 22241aba6564..2bcf01fde143 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -2863,7 +2863,7 @@ bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno)
   * This function always allocates a used segment(from dirty seglist) by SSR
   * manner, so it should recover the existing segment information of valid blocks
   */
-static void change_curseg(struct f2fs_sb_info *sbi, int type)
+static int change_curseg(struct f2fs_sb_info *sbi, int type)
  {
  	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
  	struct curseg_info *curseg = CURSEG_I(sbi, type);
@@ -2888,21 +2888,24 @@ static void change_curseg(struct f2fs_sb_info *sbi, int type)
  	if (IS_ERR(sum_page)) {
  		/* GC won't be able to use stale summary pages by cp_error */
  		memset(curseg->sum_blk, 0, SUM_ENTRY_SIZE);
-		return;
+		return PTR_ERR(sum_page);
  	}
  	sum_node = (struct f2fs_summary_block *)page_address(sum_page);
  	memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE);
  	f2fs_put_page(sum_page, 1);
+
+	return 0;
  }

  static int get_ssr_segment(struct f2fs_sb_info *sbi, int type,
  				int alloc_mode, unsigned long long age);

-static void get_atssr_segment(struct f2fs_sb_info *sbi, int type,
+static int get_atssr_segment(struct f2fs_sb_info *sbi, int type,
  					int target_type, int alloc_mode,
  					unsigned long long age)
  {
  	struct curseg_info *curseg = CURSEG_I(sbi, type);
+	int ret;

  	curseg->seg_type = target_type;

@@ -2910,38 +2913,45 @@ static void get_atssr_segment(struct f2fs_sb_info *sbi, int type,
  		struct seg_entry *se = get_seg_entry(sbi, curseg->next_segno);

  		curseg->seg_type = se->type;
-		change_curseg(sbi, type);
+		ret = change_curseg(sbi, type);
  	} else {
  		/* allocate cold segment by default */
  		curseg->seg_type = CURSEG_COLD_DATA;
-		new_curseg(sbi, type, true);
+		ret = new_curseg(sbi, type, true);
  	}
  	stat_inc_seg_type(sbi, curseg);
+
+	return ret;
  }

-static void __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
+static int __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
  {
  	struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC);
+	int ret;

  	if (!sbi->am.atgc_enabled)
-		return;
+		return 0;

  	f2fs_down_read(&SM_I(sbi)->curseg_lock);

  	mutex_lock(&curseg->curseg_mutex);
  	down_write(&SIT_I(sbi)->sentry_lock);

-	get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, CURSEG_COLD_DATA, SSR, 0);
+	ret = get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC,
+					CURSEG_COLD_DATA, SSR, 0);

  	up_write(&SIT_I(sbi)->sentry_lock);
  	mutex_unlock(&curseg->curseg_mutex);

  	f2fs_up_read(&SM_I(sbi)->curseg_lock);

+	return ret;
+
  }
-void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi)
+
+int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi)
  {
-	__f2fs_init_atgc_curseg(sbi);
+	return __f2fs_init_atgc_curseg(sbi);
  }

  static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
@@ -3069,11 +3079,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
  	return false;
  }

-void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
+int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
  					unsigned int start, unsigned int end)
  {
  	struct curseg_info *curseg = CURSEG_I(sbi, type);
  	unsigned int segno;
+	int ret = 0;

  	f2fs_down_read(&SM_I(sbi)->curseg_lock);
  	mutex_lock(&curseg->curseg_mutex);
@@ -3084,9 +3095,9 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
  		goto unlock;

  	if (f2fs_need_SSR(sbi) && get_ssr_segment(sbi, type, SSR, 0))
-		change_curseg(sbi, type);
+		ret = change_curseg(sbi, type);
  	else
-		new_curseg(sbi, type, true);
+		ret = new_curseg(sbi, type, true);

  	stat_inc_seg_type(sbi, curseg);

@@ -3100,6 +3111,8 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,

  	mutex_unlock(&curseg->curseg_mutex);
  	f2fs_up_read(&SM_I(sbi)->curseg_lock);
+
+	return ret;
  }

  static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
@@ -3107,6 +3120,7 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
  {
  	struct curseg_info *curseg = CURSEG_I(sbi, type);
  	unsigned int old_segno;
+	int ret;

  	if (type == CURSEG_COLD_DATA_PINNED && !curseg->inited)
  		goto allocate;
@@ -3119,8 +3133,9 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,

  allocate:
  	old_segno = curseg->segno;
-	if (new_curseg(sbi, type, true))
-		return -EAGAIN;
+	ret = new_curseg(sbi, type, true);
+	if (ret)
+		return ret;
  	stat_inc_seg_type(sbi, curseg);
  	locate_dirty_segment(sbi, old_segno);
  	return 0;
@@ -3480,14 +3495,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
  	bool from_gc = (type == CURSEG_ALL_DATA_ATGC);
  	struct seg_entry *se = NULL;
  	bool segment_full = false;
+	int ret = 0;

  	f2fs_down_read(&SM_I(sbi)->curseg_lock);

  	mutex_lock(&curseg->curseg_mutex);
  	down_write(&sit_i->sentry_lock);

-	if (curseg->segno == NULL_SEGNO)
+	if (curseg->segno == NULL_SEGNO) {
+		ret = -ENOSPC;
  		goto out_err;
+	}

  	if (from_gc) {
  		f2fs_bug_on(sbi, GET_SEGNO(sbi, old_blkaddr) == NULL_SEGNO);
@@ -3541,17 +3559,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
  		}

  		if (from_gc) {
-			get_atssr_segment(sbi, type, se->type,
+			ret = get_atssr_segment(sbi, type, se->type,
  						AT_SSR, se->mtime);
  		} else {
  			if (need_new_seg(sbi, type))
-				new_curseg(sbi, type, false);
+				ret = new_curseg(sbi, type, false);
  			else
-				change_curseg(sbi, type);
+				ret = change_curseg(sbi, type);
  			stat_inc_seg_type(sbi, curseg);
  		}

-		if (curseg->segno == NULL_SEGNO)
+		if (ret)
  			goto out_err;
  	}

@@ -3594,7 +3612,7 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
  	up_write(&sit_i->sentry_lock);
  	mutex_unlock(&curseg->curseg_mutex);
  	f2fs_up_read(&SM_I(sbi)->curseg_lock);
-	return -ENOSPC;
+	return ret;

  }

@@ -3824,7 +3842,8 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
  	/* change the current segment */
  	if (segno != curseg->segno) {
  		curseg->next_segno = segno;
-		change_curseg(sbi, type);
+		if (change_curseg(sbi, type))
+			goto out_unlock;
  	}

  	curseg->next_blkoff = GET_BLKOFF_FROM_SEG0(sbi, new_blkaddr);
@@ -3850,12 +3869,14 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
  	if (recover_curseg) {
  		if (old_cursegno != curseg->segno) {
  			curseg->next_segno = old_cursegno;
-			change_curseg(sbi, type);
+			if (change_curseg(sbi, type))
+				goto out_unlock;
  		}
  		curseg->next_blkoff = old_blkoff;
  		curseg->alloc_type = old_alloc_type;
  	}

+out_unlock:
  	up_write(&sit_i->sentry_lock);
  	mutex_unlock(&curseg->curseg_mutex);
  	f2fs_up_write(&SM_I(sbi)->curseg_lock);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 0676c2dcbbf7..8c69257db8cc 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4691,7 +4691,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
  	if (err)
  		goto free_meta;

-	f2fs_init_inmem_curseg(sbi);
+	err = f2fs_init_inmem_curseg(sbi);
+	if (err)
+		goto sync_free_meta;

  	/* f2fs_recover_fsync_data() cleared this already */
  	clear_sbi_flag(sbi, SBI_POR_DOING);
Zhiguo Niu March 4, 2024, 7:52 a.m. UTC | #4 
On Mon, Mar 4, 2024 at 3:17 PM Chao Yu <chao@kernel.org> wrote:
>
> On 2024/3/4 11:33, Zhiguo Niu wrote:
> > On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote:
> >>
> >> On 2024/3/1 19:36, Zhiguo Niu wrote:
> >>> new_curseg may return error if get_new_segment fail, so its result
> >>> should be check in its caller f2fs_allocate_segment_for_resize,
> >>> alos pass this results to free_segment_range.
> >>
> >> Zhiguo,
> >>
> >> What about handling all error paths of new_curseg() and change_curseg()
> >> in one patch?
> > Dear Chao,
> >
> > Do you mean to merge it with the previous patch “f2fs: fix to check
> > return value of f2fs_gc_range”?
> > Because in addition to new_curseg/change_curseg error handling, there
> > are some other changes in the previous patch.
> > besides, I searched for new related codes, and there should be the
> > only place left without error handling about new_curseg/
> > change_curseg .
>
> Zhiguo, I meant something like this?
>
> Subject: [PATCH] f2fs: fix to handle error paths of {new,change}_curseg()
Dear Chao,
I got your meaning and I think this patch looks good.
Please ignore my patch and use your version:）.
thanks!
>
> ---
>   fs/f2fs/f2fs.h    |  4 +--
>   fs/f2fs/gc.c      |  7 +++--
>   fs/f2fs/segment.c | 67 +++++++++++++++++++++++++++++++----------------
>   fs/f2fs/super.c   |  4 ++-
>   4 files changed, 54 insertions(+), 28 deletions(-)
>
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 80789255bf68..03927f1b2ea1 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -3702,10 +3702,10 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable);
>   void f2fs_release_discard_addrs(struct f2fs_sb_info *sbi);
>   int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra);
>   bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno);
> -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
> +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
>   void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
>   void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>                                         unsigned int start, unsigned int end);
>   int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
>   int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index f8314765246a..854ad0a3f6ea 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -2033,8 +2033,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
>         mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
>
>         /* Move out cursegs from the target range */
> -       for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
> -               f2fs_allocate_segment_for_resize(sbi, type, start, end);
> +       for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
> +               err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
> +               if (err)
> +                       goto out;
> +       }
>
>         /* do GC to move out valid blocks in the range */
>         err = f2fs_gc_range(sbi, start, end, dry_run, 0);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 22241aba6564..2bcf01fde143 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -2863,7 +2863,7 @@ bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno)
>    * This function always allocates a used segment(from dirty seglist) by SSR
>    * manner, so it should recover the existing segment information of valid blocks
>    */
> -static void change_curseg(struct f2fs_sb_info *sbi, int type)
> +static int change_curseg(struct f2fs_sb_info *sbi, int type)
>   {
>         struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
>         struct curseg_info *curseg = CURSEG_I(sbi, type);
> @@ -2888,21 +2888,24 @@ static void change_curseg(struct f2fs_sb_info *sbi, int type)
>         if (IS_ERR(sum_page)) {
>                 /* GC won't be able to use stale summary pages by cp_error */
>                 memset(curseg->sum_blk, 0, SUM_ENTRY_SIZE);
> -               return;
> +               return PTR_ERR(sum_page);
>         }
>         sum_node = (struct f2fs_summary_block *)page_address(sum_page);
>         memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE);
>         f2fs_put_page(sum_page, 1);
> +
> +       return 0;
>   }
>
>   static int get_ssr_segment(struct f2fs_sb_info *sbi, int type,
>                                 int alloc_mode, unsigned long long age);
>
> -static void get_atssr_segment(struct f2fs_sb_info *sbi, int type,
> +static int get_atssr_segment(struct f2fs_sb_info *sbi, int type,
>                                         int target_type, int alloc_mode,
>                                         unsigned long long age)
>   {
>         struct curseg_info *curseg = CURSEG_I(sbi, type);
> +       int ret;
>
>         curseg->seg_type = target_type;
>
> @@ -2910,38 +2913,45 @@ static void get_atssr_segment(struct f2fs_sb_info *sbi, int type,
>                 struct seg_entry *se = get_seg_entry(sbi, curseg->next_segno);
>
>                 curseg->seg_type = se->type;
> -               change_curseg(sbi, type);
> +               ret = change_curseg(sbi, type);
>         } else {
>                 /* allocate cold segment by default */
>                 curseg->seg_type = CURSEG_COLD_DATA;
> -               new_curseg(sbi, type, true);
> +               ret = new_curseg(sbi, type, true);
>         }
>         stat_inc_seg_type(sbi, curseg);
> +
> +       return ret;
>   }
>
> -static void __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
> +static int __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
>   {
>         struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC);
> +       int ret;
>
>         if (!sbi->am.atgc_enabled)
> -               return;
> +               return 0;
>
>         f2fs_down_read(&SM_I(sbi)->curseg_lock);
>
>         mutex_lock(&curseg->curseg_mutex);
>         down_write(&SIT_I(sbi)->sentry_lock);
>
> -       get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, CURSEG_COLD_DATA, SSR, 0);
> +       ret = get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC,
> +                                       CURSEG_COLD_DATA, SSR, 0);
>
>         up_write(&SIT_I(sbi)->sentry_lock);
>         mutex_unlock(&curseg->curseg_mutex);
>
>         f2fs_up_read(&SM_I(sbi)->curseg_lock);
>
> +       return ret;
> +
>   }
> -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi)
> +
> +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi)
>   {
> -       __f2fs_init_atgc_curseg(sbi);
> +       return __f2fs_init_atgc_curseg(sbi);
>   }
>
>   static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
> @@ -3069,11 +3079,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
>         return false;
>   }
>
> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>                                         unsigned int start, unsigned int end)
>   {
>         struct curseg_info *curseg = CURSEG_I(sbi, type);
>         unsigned int segno;
> +       int ret = 0;
>
>         f2fs_down_read(&SM_I(sbi)->curseg_lock);
>         mutex_lock(&curseg->curseg_mutex);
> @@ -3084,9 +3095,9 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>                 goto unlock;
>
>         if (f2fs_need_SSR(sbi) && get_ssr_segment(sbi, type, SSR, 0))
> -               change_curseg(sbi, type);
> +               ret = change_curseg(sbi, type);
>         else
> -               new_curseg(sbi, type, true);
> +               ret = new_curseg(sbi, type, true);
>
>         stat_inc_seg_type(sbi, curseg);
>
> @@ -3100,6 +3111,8 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>
>         mutex_unlock(&curseg->curseg_mutex);
>         f2fs_up_read(&SM_I(sbi)->curseg_lock);
> +
> +       return ret;
>   }
>
>   static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
> @@ -3107,6 +3120,7 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
>   {
>         struct curseg_info *curseg = CURSEG_I(sbi, type);
>         unsigned int old_segno;
> +       int ret;
>
>         if (type == CURSEG_COLD_DATA_PINNED && !curseg->inited)
>                 goto allocate;
> @@ -3119,8 +3133,9 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
>
>   allocate:
>         old_segno = curseg->segno;
> -       if (new_curseg(sbi, type, true))
> -               return -EAGAIN;
> +       ret = new_curseg(sbi, type, true);
> +       if (ret)
> +               return ret;
>         stat_inc_seg_type(sbi, curseg);
>         locate_dirty_segment(sbi, old_segno);
>         return 0;
> @@ -3480,14 +3495,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>         bool from_gc = (type == CURSEG_ALL_DATA_ATGC);
>         struct seg_entry *se = NULL;
>         bool segment_full = false;
> +       int ret = 0;
>
>         f2fs_down_read(&SM_I(sbi)->curseg_lock);
>
>         mutex_lock(&curseg->curseg_mutex);
>         down_write(&sit_i->sentry_lock);
>
> -       if (curseg->segno == NULL_SEGNO)
> +       if (curseg->segno == NULL_SEGNO) {
> +               ret = -ENOSPC;
>                 goto out_err;
> +       }
>
>         if (from_gc) {
>                 f2fs_bug_on(sbi, GET_SEGNO(sbi, old_blkaddr) == NULL_SEGNO);
> @@ -3541,17 +3559,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>                 }
>
>                 if (from_gc) {
> -                       get_atssr_segment(sbi, type, se->type,
> +                       ret = get_atssr_segment(sbi, type, se->type,
>                                                 AT_SSR, se->mtime);
>                 } else {
>                         if (need_new_seg(sbi, type))
> -                               new_curseg(sbi, type, false);
> +                               ret = new_curseg(sbi, type, false);
>                         else
> -                               change_curseg(sbi, type);
> +                               ret = change_curseg(sbi, type);
>                         stat_inc_seg_type(sbi, curseg);
>                 }
>
> -               if (curseg->segno == NULL_SEGNO)
> +               if (ret)
>                         goto out_err;
>         }
>
> @@ -3594,7 +3612,7 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>         up_write(&sit_i->sentry_lock);
>         mutex_unlock(&curseg->curseg_mutex);
>         f2fs_up_read(&SM_I(sbi)->curseg_lock);
> -       return -ENOSPC;
> +       return ret;
>
>   }
>
> @@ -3824,7 +3842,8 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
>         /* change the current segment */
>         if (segno != curseg->segno) {
>                 curseg->next_segno = segno;
> -               change_curseg(sbi, type);
> +               if (change_curseg(sbi, type))
> +                       goto out_unlock;
>         }
>
>         curseg->next_blkoff = GET_BLKOFF_FROM_SEG0(sbi, new_blkaddr);
> @@ -3850,12 +3869,14 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
>         if (recover_curseg) {
>                 if (old_cursegno != curseg->segno) {
>                         curseg->next_segno = old_cursegno;
> -                       change_curseg(sbi, type);
> +                       if (change_curseg(sbi, type))
> +                               goto out_unlock;
>                 }
>                 curseg->next_blkoff = old_blkoff;
>                 curseg->alloc_type = old_alloc_type;
>         }
>
> +out_unlock:
>         up_write(&sit_i->sentry_lock);
>         mutex_unlock(&curseg->curseg_mutex);
>         f2fs_up_write(&SM_I(sbi)->curseg_lock);
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 0676c2dcbbf7..8c69257db8cc 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -4691,7 +4691,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>         if (err)
>                 goto free_meta;
>
> -       f2fs_init_inmem_curseg(sbi);
> +       err = f2fs_init_inmem_curseg(sbi);
> +       if (err)
> +               goto sync_free_meta;
>
>         /* f2fs_recover_fsync_data() cleared this already */
>         clear_sbi_flag(sbi, SBI_POR_DOING);
> --
> 2.40.1
>
>
>
> >
> > thanks!
> >>
> >> Thanks,
> >>
> >>>
> >>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
> >>> ---
> >>>    fs/f2fs/f2fs.h    | 2 +-
> >>>    fs/f2fs/gc.c      | 7 +++++--
> >>>    fs/f2fs/segment.c | 9 +++++++--
> >>>    3 files changed, 13 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >>> index 4331012..39dda7d 100644
> >>> --- a/fs/f2fs/f2fs.h
> >>> +++ b/fs/f2fs/f2fs.h
> >>> @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
> >>>    void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
> >>>    void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
> >>>    void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
> >>> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>                                        unsigned int start, unsigned int end);
> >>>    int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
> >>>    int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
> >>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >>> index c60b747..7a458fa 100644
> >>> --- a/fs/f2fs/gc.c
> >>> +++ b/fs/f2fs/gc.c
> >>> @@ -2037,8 +2037,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
> >>>        mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
> >>>
> >>>        /* Move out cursegs from the target range */
> >>> -     for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
> >>> -             f2fs_allocate_segment_for_resize(sbi, type, start, end);
> >>> +     for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
> >>> +             err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
> >>> +             if (err)
> >>> +                     goto out;
> >>> +     }
> >>>
> >>>        /* do GC to move out valid blocks in the range */
> >>>        err = f2fs_gc_range(sbi, start, end, dry_run, 0);
> >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >>> index 1bb3019..2a07b9d 100644
> >>> --- a/fs/f2fs/segment.c
> >>> +++ b/fs/f2fs/segment.c
> >>> @@ -3071,11 +3071,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
> >>>        return false;
> >>>    }
> >>>
> >>> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>                                        unsigned int start, unsigned int end)
> >>>    {
> >>>        struct curseg_info *curseg = CURSEG_I(sbi, type);
> >>>        unsigned int segno;
> >>> +     int err = 0;
> >>>
> >>>        f2fs_down_read(&SM_I(sbi)->curseg_lock);
> >>>        mutex_lock(&curseg->curseg_mutex);
> >>> @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>                change_curseg(sbi, type);
> >>>        else
> >>>                new_curseg(sbi, type, true);
> >>> -
> >>> +     if (curseg->segno == NULL_SEGNO) {
> >>> +             err = -ENOSPC;
> >>> +             goto unlock;
> >>> +     }
> >>>        stat_inc_seg_type(sbi, curseg);
> >>>
> >>>        locate_dirty_segment(sbi, segno);
> >>> @@ -3102,6 +3106,7 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>
> >>>        mutex_unlock(&curseg->curseg_mutex);
> >>>        f2fs_up_read(&SM_I(sbi)->curseg_lock);
> >>> +     return err;
> >>>    }
> >>>
> >>>    static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
Chao Yu March 4, 2024, 9:19 a.m. UTC | #5 
On 2024/3/4 15:52, Zhiguo Niu wrote:
> On Mon, Mar 4, 2024 at 3:17 PM Chao Yu <chao@kernel.org> wrote:
>>
>> On 2024/3/4 11:33, Zhiguo Niu wrote:
>>> On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote:
>>>>
>>>> On 2024/3/1 19:36, Zhiguo Niu wrote:
>>>>> new_curseg may return error if get_new_segment fail, so its result
>>>>> should be check in its caller f2fs_allocate_segment_for_resize,
>>>>> alos pass this results to free_segment_range.
>>>>
>>>> Zhiguo,
>>>>
>>>> What about handling all error paths of new_curseg() and change_curseg()
>>>> in one patch?
>>> Dear Chao,
>>>
>>> Do you mean to merge it with the previous patch “f2fs: fix to check
>>> return value of f2fs_gc_range”?
>>> Because in addition to new_curseg/change_curseg error handling, there
>>> are some other changes in the previous patch.
>>> besides, I searched for new related codes, and there should be the
>>> only place left without error handling about new_curseg/
>>> change_curseg .
>>
>> Zhiguo, I meant something like this?
>>
>> Subject: [PATCH] f2fs: fix to handle error paths of {new,change}_curseg()
> Dear Chao,
> I got your meaning and I think this patch looks good.
> Please ignore my patch and use your version:）.

Zhiguo,

This is a raw patch, can you please write commit message for it, and
resend it once you've tested it.

Thanks,

> thanks!
>>
>> ---
>>    fs/f2fs/f2fs.h    |  4 +--
>>    fs/f2fs/gc.c      |  7 +++--
>>    fs/f2fs/segment.c | 67 +++++++++++++++++++++++++++++++----------------
>>    fs/f2fs/super.c   |  4 ++-
>>    4 files changed, 54 insertions(+), 28 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 80789255bf68..03927f1b2ea1 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -3702,10 +3702,10 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable);
>>    void f2fs_release_discard_addrs(struct f2fs_sb_info *sbi);
>>    int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra);
>>    bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno);
>> -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
>> +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
>>    void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
>>    void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
>> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>                                          unsigned int start, unsigned int end);
>>    int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
>>    int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index f8314765246a..854ad0a3f6ea 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -2033,8 +2033,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
>>          mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
>>
>>          /* Move out cursegs from the target range */
>> -       for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
>> -               f2fs_allocate_segment_for_resize(sbi, type, start, end);
>> +       for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
>> +               err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
>> +               if (err)
>> +                       goto out;
>> +       }
>>
>>          /* do GC to move out valid blocks in the range */
>>          err = f2fs_gc_range(sbi, start, end, dry_run, 0);
>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>> index 22241aba6564..2bcf01fde143 100644
>> --- a/fs/f2fs/segment.c
>> +++ b/fs/f2fs/segment.c
>> @@ -2863,7 +2863,7 @@ bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno)
>>     * This function always allocates a used segment(from dirty seglist) by SSR
>>     * manner, so it should recover the existing segment information of valid blocks
>>     */
>> -static void change_curseg(struct f2fs_sb_info *sbi, int type)
>> +static int change_curseg(struct f2fs_sb_info *sbi, int type)
>>    {
>>          struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
>>          struct curseg_info *curseg = CURSEG_I(sbi, type);
>> @@ -2888,21 +2888,24 @@ static void change_curseg(struct f2fs_sb_info *sbi, int type)
>>          if (IS_ERR(sum_page)) {
>>                  /* GC won't be able to use stale summary pages by cp_error */
>>                  memset(curseg->sum_blk, 0, SUM_ENTRY_SIZE);
>> -               return;
>> +               return PTR_ERR(sum_page);
>>          }
>>          sum_node = (struct f2fs_summary_block *)page_address(sum_page);
>>          memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE);
>>          f2fs_put_page(sum_page, 1);
>> +
>> +       return 0;
>>    }
>>
>>    static int get_ssr_segment(struct f2fs_sb_info *sbi, int type,
>>                                  int alloc_mode, unsigned long long age);
>>
>> -static void get_atssr_segment(struct f2fs_sb_info *sbi, int type,
>> +static int get_atssr_segment(struct f2fs_sb_info *sbi, int type,
>>                                          int target_type, int alloc_mode,
>>                                          unsigned long long age)
>>    {
>>          struct curseg_info *curseg = CURSEG_I(sbi, type);
>> +       int ret;
>>
>>          curseg->seg_type = target_type;
>>
>> @@ -2910,38 +2913,45 @@ static void get_atssr_segment(struct f2fs_sb_info *sbi, int type,
>>                  struct seg_entry *se = get_seg_entry(sbi, curseg->next_segno);
>>
>>                  curseg->seg_type = se->type;
>> -               change_curseg(sbi, type);
>> +               ret = change_curseg(sbi, type);
>>          } else {
>>                  /* allocate cold segment by default */
>>                  curseg->seg_type = CURSEG_COLD_DATA;
>> -               new_curseg(sbi, type, true);
>> +               ret = new_curseg(sbi, type, true);
>>          }
>>          stat_inc_seg_type(sbi, curseg);
>> +
>> +       return ret;
>>    }
>>
>> -static void __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
>> +static int __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
>>    {
>>          struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC);
>> +       int ret;
>>
>>          if (!sbi->am.atgc_enabled)
>> -               return;
>> +               return 0;
>>
>>          f2fs_down_read(&SM_I(sbi)->curseg_lock);
>>
>>          mutex_lock(&curseg->curseg_mutex);
>>          down_write(&SIT_I(sbi)->sentry_lock);
>>
>> -       get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, CURSEG_COLD_DATA, SSR, 0);
>> +       ret = get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC,
>> +                                       CURSEG_COLD_DATA, SSR, 0);
>>
>>          up_write(&SIT_I(sbi)->sentry_lock);
>>          mutex_unlock(&curseg->curseg_mutex);
>>
>>          f2fs_up_read(&SM_I(sbi)->curseg_lock);
>>
>> +       return ret;
>> +
>>    }
>> -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi)
>> +
>> +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi)
>>    {
>> -       __f2fs_init_atgc_curseg(sbi);
>> +       return __f2fs_init_atgc_curseg(sbi);
>>    }
>>
>>    static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
>> @@ -3069,11 +3079,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
>>          return false;
>>    }
>>
>> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>                                          unsigned int start, unsigned int end)
>>    {
>>          struct curseg_info *curseg = CURSEG_I(sbi, type);
>>          unsigned int segno;
>> +       int ret = 0;
>>
>>          f2fs_down_read(&SM_I(sbi)->curseg_lock);
>>          mutex_lock(&curseg->curseg_mutex);
>> @@ -3084,9 +3095,9 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>                  goto unlock;
>>
>>          if (f2fs_need_SSR(sbi) && get_ssr_segment(sbi, type, SSR, 0))
>> -               change_curseg(sbi, type);
>> +               ret = change_curseg(sbi, type);
>>          else
>> -               new_curseg(sbi, type, true);
>> +               ret = new_curseg(sbi, type, true);
>>
>>          stat_inc_seg_type(sbi, curseg);
>>
>> @@ -3100,6 +3111,8 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>
>>          mutex_unlock(&curseg->curseg_mutex);
>>          f2fs_up_read(&SM_I(sbi)->curseg_lock);
>> +
>> +       return ret;
>>    }
>>
>>    static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
>> @@ -3107,6 +3120,7 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
>>    {
>>          struct curseg_info *curseg = CURSEG_I(sbi, type);
>>          unsigned int old_segno;
>> +       int ret;
>>
>>          if (type == CURSEG_COLD_DATA_PINNED && !curseg->inited)
>>                  goto allocate;
>> @@ -3119,8 +3133,9 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
>>
>>    allocate:
>>          old_segno = curseg->segno;
>> -       if (new_curseg(sbi, type, true))
>> -               return -EAGAIN;
>> +       ret = new_curseg(sbi, type, true);
>> +       if (ret)
>> +               return ret;
>>          stat_inc_seg_type(sbi, curseg);
>>          locate_dirty_segment(sbi, old_segno);
>>          return 0;
>> @@ -3480,14 +3495,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>>          bool from_gc = (type == CURSEG_ALL_DATA_ATGC);
>>          struct seg_entry *se = NULL;
>>          bool segment_full = false;
>> +       int ret = 0;
>>
>>          f2fs_down_read(&SM_I(sbi)->curseg_lock);
>>
>>          mutex_lock(&curseg->curseg_mutex);
>>          down_write(&sit_i->sentry_lock);
>>
>> -       if (curseg->segno == NULL_SEGNO)
>> +       if (curseg->segno == NULL_SEGNO) {
>> +               ret = -ENOSPC;
>>                  goto out_err;
>> +       }
>>
>>          if (from_gc) {
>>                  f2fs_bug_on(sbi, GET_SEGNO(sbi, old_blkaddr) == NULL_SEGNO);
>> @@ -3541,17 +3559,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>>                  }
>>
>>                  if (from_gc) {
>> -                       get_atssr_segment(sbi, type, se->type,
>> +                       ret = get_atssr_segment(sbi, type, se->type,
>>                                                  AT_SSR, se->mtime);
>>                  } else {
>>                          if (need_new_seg(sbi, type))
>> -                               new_curseg(sbi, type, false);
>> +                               ret = new_curseg(sbi, type, false);
>>                          else
>> -                               change_curseg(sbi, type);
>> +                               ret = change_curseg(sbi, type);
>>                          stat_inc_seg_type(sbi, curseg);
>>                  }
>>
>> -               if (curseg->segno == NULL_SEGNO)
>> +               if (ret)
>>                          goto out_err;
>>          }
>>
>> @@ -3594,7 +3612,7 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>>          up_write(&sit_i->sentry_lock);
>>          mutex_unlock(&curseg->curseg_mutex);
>>          f2fs_up_read(&SM_I(sbi)->curseg_lock);
>> -       return -ENOSPC;
>> +       return ret;
>>
>>    }
>>
>> @@ -3824,7 +3842,8 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
>>          /* change the current segment */
>>          if (segno != curseg->segno) {
>>                  curseg->next_segno = segno;
>> -               change_curseg(sbi, type);
>> +               if (change_curseg(sbi, type))
>> +                       goto out_unlock;
>>          }
>>
>>          curseg->next_blkoff = GET_BLKOFF_FROM_SEG0(sbi, new_blkaddr);
>> @@ -3850,12 +3869,14 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
>>          if (recover_curseg) {
>>                  if (old_cursegno != curseg->segno) {
>>                          curseg->next_segno = old_cursegno;
>> -                       change_curseg(sbi, type);
>> +                       if (change_curseg(sbi, type))
>> +                               goto out_unlock;
>>                  }
>>                  curseg->next_blkoff = old_blkoff;
>>                  curseg->alloc_type = old_alloc_type;
>>          }
>>
>> +out_unlock:
>>          up_write(&sit_i->sentry_lock);
>>          mutex_unlock(&curseg->curseg_mutex);
>>          f2fs_up_write(&SM_I(sbi)->curseg_lock);
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index 0676c2dcbbf7..8c69257db8cc 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -4691,7 +4691,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>>          if (err)
>>                  goto free_meta;
>>
>> -       f2fs_init_inmem_curseg(sbi);
>> +       err = f2fs_init_inmem_curseg(sbi);
>> +       if (err)
>> +               goto sync_free_meta;
>>
>>          /* f2fs_recover_fsync_data() cleared this already */
>>          clear_sbi_flag(sbi, SBI_POR_DOING);
>> --
>> 2.40.1
>>
>>
>>
>>>
>>> thanks!
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
>>>>> ---
>>>>>     fs/f2fs/f2fs.h    | 2 +-
>>>>>     fs/f2fs/gc.c      | 7 +++++--
>>>>>     fs/f2fs/segment.c | 9 +++++++--
>>>>>     3 files changed, 13 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>>>>> index 4331012..39dda7d 100644
>>>>> --- a/fs/f2fs/f2fs.h
>>>>> +++ b/fs/f2fs/f2fs.h
>>>>> @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
>>>>>     void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
>>>>>     void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
>>>>>     void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
>>>>> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>>>> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>>>>                                         unsigned int start, unsigned int end);
>>>>>     int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
>>>>>     int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
>>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>>> index c60b747..7a458fa 100644
>>>>> --- a/fs/f2fs/gc.c
>>>>> +++ b/fs/f2fs/gc.c
>>>>> @@ -2037,8 +2037,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
>>>>>         mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
>>>>>
>>>>>         /* Move out cursegs from the target range */
>>>>> -     for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
>>>>> -             f2fs_allocate_segment_for_resize(sbi, type, start, end);
>>>>> +     for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
>>>>> +             err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
>>>>> +             if (err)
>>>>> +                     goto out;
>>>>> +     }
>>>>>
>>>>>         /* do GC to move out valid blocks in the range */
>>>>>         err = f2fs_gc_range(sbi, start, end, dry_run, 0);
>>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>>>> index 1bb3019..2a07b9d 100644
>>>>> --- a/fs/f2fs/segment.c
>>>>> +++ b/fs/f2fs/segment.c
>>>>> @@ -3071,11 +3071,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
>>>>>         return false;
>>>>>     }
>>>>>
>>>>> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>>>> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>>>>                                         unsigned int start, unsigned int end)
>>>>>     {
>>>>>         struct curseg_info *curseg = CURSEG_I(sbi, type);
>>>>>         unsigned int segno;
>>>>> +     int err = 0;
>>>>>
>>>>>         f2fs_down_read(&SM_I(sbi)->curseg_lock);
>>>>>         mutex_lock(&curseg->curseg_mutex);
>>>>> @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>>>>                 change_curseg(sbi, type);
>>>>>         else
>>>>>                 new_curseg(sbi, type, true);
>>>>> -
>>>>> +     if (curseg->segno == NULL_SEGNO) {
>>>>> +             err = -ENOSPC;
>>>>> +             goto unlock;
>>>>> +     }
>>>>>         stat_inc_seg_type(sbi, curseg);
>>>>>
>>>>>         locate_dirty_segment(sbi, segno);
>>>>> @@ -3102,6 +3106,7 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
>>>>>
>>>>>         mutex_unlock(&curseg->curseg_mutex);
>>>>>         f2fs_up_read(&SM_I(sbi)->curseg_lock);
>>>>> +     return err;
>>>>>     }
>>>>>
>>>>>     static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
Zhiguo Niu March 4, 2024, 10:43 a.m. UTC | #6 
On Mon, Mar 4, 2024 at 5:19 PM Chao Yu <chao@kernel.org> wrote:
>
> On 2024/3/4 15:52, Zhiguo Niu wrote:
> > On Mon, Mar 4, 2024 at 3:17 PM Chao Yu <chao@kernel.org> wrote:
> >>
> >> On 2024/3/4 11:33, Zhiguo Niu wrote:
> >>> On Mon, Mar 4, 2024 at 11:19 AM Chao Yu <chao@kernel.org> wrote:
> >>>>
> >>>> On 2024/3/1 19:36, Zhiguo Niu wrote:
> >>>>> new_curseg may return error if get_new_segment fail, so its result
> >>>>> should be check in its caller f2fs_allocate_segment_for_resize,
> >>>>> alos pass this results to free_segment_range.
> >>>>
> >>>> Zhiguo,
> >>>>
> >>>> What about handling all error paths of new_curseg() and change_curseg()
> >>>> in one patch?
> >>> Dear Chao,
> >>>
> >>> Do you mean to merge it with the previous patch “f2fs: fix to check
> >>> return value of f2fs_gc_range”?
> >>> Because in addition to new_curseg/change_curseg error handling, there
> >>> are some other changes in the previous patch.
> >>> besides, I searched for new related codes, and there should be the
> >>> only place left without error handling about new_curseg/
> >>> change_curseg .
> >>
> >> Zhiguo, I meant something like this?
> >>
> >> Subject: [PATCH] f2fs: fix to handle error paths of {new,change}_curseg()
> > Dear Chao,
> > I got your meaning and I think this patch looks good.
> > Please ignore my patch and use your version:）.
>
> Zhiguo,
>
> This is a raw patch, can you please write commit message for it, and
> resend it once you've tested it.
Dear Chao,
OK, no problem.
thanks!
>
> Thanks,
>
> > thanks!
> >>
> >> ---
> >>    fs/f2fs/f2fs.h    |  4 +--
> >>    fs/f2fs/gc.c      |  7 +++--
> >>    fs/f2fs/segment.c | 67 +++++++++++++++++++++++++++++++----------------
> >>    fs/f2fs/super.c   |  4 ++-
> >>    4 files changed, 54 insertions(+), 28 deletions(-)
> >>
> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >> index 80789255bf68..03927f1b2ea1 100644
> >> --- a/fs/f2fs/f2fs.h
> >> +++ b/fs/f2fs/f2fs.h
> >> @@ -3702,10 +3702,10 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t unusable);
> >>    void f2fs_release_discard_addrs(struct f2fs_sb_info *sbi);
> >>    int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra);
> >>    bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno);
> >> -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
> >> +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
> >>    void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
> >>    void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
> >> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>                                          unsigned int start, unsigned int end);
> >>    int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
> >>    int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >> index f8314765246a..854ad0a3f6ea 100644
> >> --- a/fs/f2fs/gc.c
> >> +++ b/fs/f2fs/gc.c
> >> @@ -2033,8 +2033,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
> >>          mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
> >>
> >>          /* Move out cursegs from the target range */
> >> -       for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
> >> -               f2fs_allocate_segment_for_resize(sbi, type, start, end);
> >> +       for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
> >> +               err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
> >> +               if (err)
> >> +                       goto out;
> >> +       }
> >>
> >>          /* do GC to move out valid blocks in the range */
> >>          err = f2fs_gc_range(sbi, start, end, dry_run, 0);
> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >> index 22241aba6564..2bcf01fde143 100644
> >> --- a/fs/f2fs/segment.c
> >> +++ b/fs/f2fs/segment.c
> >> @@ -2863,7 +2863,7 @@ bool f2fs_segment_has_free_slot(struct f2fs_sb_info *sbi, int segno)
> >>     * This function always allocates a used segment(from dirty seglist) by SSR
> >>     * manner, so it should recover the existing segment information of valid blocks
> >>     */
> >> -static void change_curseg(struct f2fs_sb_info *sbi, int type)
> >> +static int change_curseg(struct f2fs_sb_info *sbi, int type)
> >>    {
> >>          struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
> >>          struct curseg_info *curseg = CURSEG_I(sbi, type);
> >> @@ -2888,21 +2888,24 @@ static void change_curseg(struct f2fs_sb_info *sbi, int type)
> >>          if (IS_ERR(sum_page)) {
> >>                  /* GC won't be able to use stale summary pages by cp_error */
> >>                  memset(curseg->sum_blk, 0, SUM_ENTRY_SIZE);
> >> -               return;
> >> +               return PTR_ERR(sum_page);
> >>          }
> >>          sum_node = (struct f2fs_summary_block *)page_address(sum_page);
> >>          memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE);
> >>          f2fs_put_page(sum_page, 1);
> >> +
> >> +       return 0;
> >>    }
> >>
> >>    static int get_ssr_segment(struct f2fs_sb_info *sbi, int type,
> >>                                  int alloc_mode, unsigned long long age);
> >>
> >> -static void get_atssr_segment(struct f2fs_sb_info *sbi, int type,
> >> +static int get_atssr_segment(struct f2fs_sb_info *sbi, int type,
> >>                                          int target_type, int alloc_mode,
> >>                                          unsigned long long age)
> >>    {
> >>          struct curseg_info *curseg = CURSEG_I(sbi, type);
> >> +       int ret;
> >>
> >>          curseg->seg_type = target_type;
> >>
> >> @@ -2910,38 +2913,45 @@ static void get_atssr_segment(struct f2fs_sb_info *sbi, int type,
> >>                  struct seg_entry *se = get_seg_entry(sbi, curseg->next_segno);
> >>
> >>                  curseg->seg_type = se->type;
> >> -               change_curseg(sbi, type);
> >> +               ret = change_curseg(sbi, type);
> >>          } else {
> >>                  /* allocate cold segment by default */
> >>                  curseg->seg_type = CURSEG_COLD_DATA;
> >> -               new_curseg(sbi, type, true);
> >> +               ret = new_curseg(sbi, type, true);
> >>          }
> >>          stat_inc_seg_type(sbi, curseg);
> >> +
> >> +       return ret;
> >>    }
> >>
> >> -static void __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
> >> +static int __f2fs_init_atgc_curseg(struct f2fs_sb_info *sbi)
> >>    {
> >>          struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_ALL_DATA_ATGC);
> >> +       int ret;
> >>
> >>          if (!sbi->am.atgc_enabled)
> >> -               return;
> >> +               return 0;
> >>
> >>          f2fs_down_read(&SM_I(sbi)->curseg_lock);
> >>
> >>          mutex_lock(&curseg->curseg_mutex);
> >>          down_write(&SIT_I(sbi)->sentry_lock);
> >>
> >> -       get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC, CURSEG_COLD_DATA, SSR, 0);
> >> +       ret = get_atssr_segment(sbi, CURSEG_ALL_DATA_ATGC,
> >> +                                       CURSEG_COLD_DATA, SSR, 0);
> >>
> >>          up_write(&SIT_I(sbi)->sentry_lock);
> >>          mutex_unlock(&curseg->curseg_mutex);
> >>
> >>          f2fs_up_read(&SM_I(sbi)->curseg_lock);
> >>
> >> +       return ret;
> >> +
> >>    }
> >> -void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi)
> >> +
> >> +int f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi)
> >>    {
> >> -       __f2fs_init_atgc_curseg(sbi);
> >> +       return __f2fs_init_atgc_curseg(sbi);
> >>    }
> >>
> >>    static void __f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi, int type)
> >> @@ -3069,11 +3079,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
> >>          return false;
> >>    }
> >>
> >> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>                                          unsigned int start, unsigned int end)
> >>    {
> >>          struct curseg_info *curseg = CURSEG_I(sbi, type);
> >>          unsigned int segno;
> >> +       int ret = 0;
> >>
> >>          f2fs_down_read(&SM_I(sbi)->curseg_lock);
> >>          mutex_lock(&curseg->curseg_mutex);
> >> @@ -3084,9 +3095,9 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>                  goto unlock;
> >>
> >>          if (f2fs_need_SSR(sbi) && get_ssr_segment(sbi, type, SSR, 0))
> >> -               change_curseg(sbi, type);
> >> +               ret = change_curseg(sbi, type);
> >>          else
> >> -               new_curseg(sbi, type, true);
> >> +               ret = new_curseg(sbi, type, true);
> >>
> >>          stat_inc_seg_type(sbi, curseg);
> >>
> >> @@ -3100,6 +3111,8 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>
> >>          mutex_unlock(&curseg->curseg_mutex);
> >>          f2fs_up_read(&SM_I(sbi)->curseg_lock);
> >> +
> >> +       return ret;
> >>    }
> >>
> >>    static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
> >> @@ -3107,6 +3120,7 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
> >>    {
> >>          struct curseg_info *curseg = CURSEG_I(sbi, type);
> >>          unsigned int old_segno;
> >> +       int ret;
> >>
> >>          if (type == CURSEG_COLD_DATA_PINNED && !curseg->inited)
> >>                  goto allocate;
> >> @@ -3119,8 +3133,9 @@ static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
> >>
> >>    allocate:
> >>          old_segno = curseg->segno;
> >> -       if (new_curseg(sbi, type, true))
> >> -               return -EAGAIN;
> >> +       ret = new_curseg(sbi, type, true);
> >> +       if (ret)
> >> +               return ret;
> >>          stat_inc_seg_type(sbi, curseg);
> >>          locate_dirty_segment(sbi, old_segno);
> >>          return 0;
> >> @@ -3480,14 +3495,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
> >>          bool from_gc = (type == CURSEG_ALL_DATA_ATGC);
> >>          struct seg_entry *se = NULL;
> >>          bool segment_full = false;
> >> +       int ret = 0;
> >>
> >>          f2fs_down_read(&SM_I(sbi)->curseg_lock);
> >>
> >>          mutex_lock(&curseg->curseg_mutex);
> >>          down_write(&sit_i->sentry_lock);
> >>
> >> -       if (curseg->segno == NULL_SEGNO)
> >> +       if (curseg->segno == NULL_SEGNO) {
> >> +               ret = -ENOSPC;
> >>                  goto out_err;
> >> +       }
> >>
> >>          if (from_gc) {
> >>                  f2fs_bug_on(sbi, GET_SEGNO(sbi, old_blkaddr) == NULL_SEGNO);
> >> @@ -3541,17 +3559,17 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
> >>                  }
> >>
> >>                  if (from_gc) {
> >> -                       get_atssr_segment(sbi, type, se->type,
> >> +                       ret = get_atssr_segment(sbi, type, se->type,
> >>                                                  AT_SSR, se->mtime);
> >>                  } else {
> >>                          if (need_new_seg(sbi, type))
> >> -                               new_curseg(sbi, type, false);
> >> +                               ret = new_curseg(sbi, type, false);
> >>                          else
> >> -                               change_curseg(sbi, type);
> >> +                               ret = change_curseg(sbi, type);
> >>                          stat_inc_seg_type(sbi, curseg);
> >>                  }
> >>
> >> -               if (curseg->segno == NULL_SEGNO)
> >> +               if (ret)
> >>                          goto out_err;
> >>          }
> >>
> >> @@ -3594,7 +3612,7 @@ int f2fs_allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
> >>          up_write(&sit_i->sentry_lock);
> >>          mutex_unlock(&curseg->curseg_mutex);
> >>          f2fs_up_read(&SM_I(sbi)->curseg_lock);
> >> -       return -ENOSPC;
> >> +       return ret;
> >>
> >>    }
> >>
> >> @@ -3824,7 +3842,8 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
> >>          /* change the current segment */
> >>          if (segno != curseg->segno) {
> >>                  curseg->next_segno = segno;
> >> -               change_curseg(sbi, type);
> >> +               if (change_curseg(sbi, type))
> >> +                       goto out_unlock;
> >>          }
> >>
> >>          curseg->next_blkoff = GET_BLKOFF_FROM_SEG0(sbi, new_blkaddr);
> >> @@ -3850,12 +3869,14 @@ void f2fs_do_replace_block(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
> >>          if (recover_curseg) {
> >>                  if (old_cursegno != curseg->segno) {
> >>                          curseg->next_segno = old_cursegno;
> >> -                       change_curseg(sbi, type);
> >> +                       if (change_curseg(sbi, type))
> >> +                               goto out_unlock;
> >>                  }
> >>                  curseg->next_blkoff = old_blkoff;
> >>                  curseg->alloc_type = old_alloc_type;
> >>          }
> >>
> >> +out_unlock:
> >>          up_write(&sit_i->sentry_lock);
> >>          mutex_unlock(&curseg->curseg_mutex);
> >>          f2fs_up_write(&SM_I(sbi)->curseg_lock);
> >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> >> index 0676c2dcbbf7..8c69257db8cc 100644
> >> --- a/fs/f2fs/super.c
> >> +++ b/fs/f2fs/super.c
> >> @@ -4691,7 +4691,9 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
> >>          if (err)
> >>                  goto free_meta;
> >>
> >> -       f2fs_init_inmem_curseg(sbi);
> >> +       err = f2fs_init_inmem_curseg(sbi);
> >> +       if (err)
> >> +               goto sync_free_meta;
> >>
> >>          /* f2fs_recover_fsync_data() cleared this already */
> >>          clear_sbi_flag(sbi, SBI_POR_DOING);
> >> --
> >> 2.40.1
> >>
> >>
> >>
> >>>
> >>> thanks!
> >>>>
> >>>> Thanks,
> >>>>
> >>>>>
> >>>>> Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
> >>>>> ---
> >>>>>     fs/f2fs/f2fs.h    | 2 +-
> >>>>>     fs/f2fs/gc.c      | 7 +++++--
> >>>>>     fs/f2fs/segment.c | 9 +++++++--
> >>>>>     3 files changed, 13 insertions(+), 5 deletions(-)
> >>>>>
> >>>>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >>>>> index 4331012..39dda7d 100644
> >>>>> --- a/fs/f2fs/f2fs.h
> >>>>> +++ b/fs/f2fs/f2fs.h
> >>>>> @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
> >>>>>     void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
> >>>>>     void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
> >>>>>     void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
> >>>>> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>>> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>>>                                         unsigned int start, unsigned int end);
> >>>>>     int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
> >>>>>     int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
> >>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >>>>> index c60b747..7a458fa 100644
> >>>>> --- a/fs/f2fs/gc.c
> >>>>> +++ b/fs/f2fs/gc.c
> >>>>> @@ -2037,8 +2037,11 @@ static int free_segment_range(struct f2fs_sb_info *sbi,
> >>>>>         mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
> >>>>>
> >>>>>         /* Move out cursegs from the target range */
> >>>>> -     for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
> >>>>> -             f2fs_allocate_segment_for_resize(sbi, type, start, end);
> >>>>> +     for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
> >>>>> +             err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
> >>>>> +             if (err)
> >>>>> +                     goto out;
> >>>>> +     }
> >>>>>
> >>>>>         /* do GC to move out valid blocks in the range */
> >>>>>         err = f2fs_gc_range(sbi, start, end, dry_run, 0);
> >>>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >>>>> index 1bb3019..2a07b9d 100644
> >>>>> --- a/fs/f2fs/segment.c
> >>>>> +++ b/fs/f2fs/segment.c
> >>>>> @@ -3071,11 +3071,12 @@ static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
> >>>>>         return false;
> >>>>>     }
> >>>>>
> >>>>> -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>>> +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>>>                                         unsigned int start, unsigned int end)
> >>>>>     {
> >>>>>         struct curseg_info *curseg = CURSEG_I(sbi, type);
> >>>>>         unsigned int segno;
> >>>>> +     int err = 0;
> >>>>>
> >>>>>         f2fs_down_read(&SM_I(sbi)->curseg_lock);
> >>>>>         mutex_lock(&curseg->curseg_mutex);
> >>>>> @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>>>                 change_curseg(sbi, type);
> >>>>>         else
> >>>>>                 new_curseg(sbi, type, true);
> >>>>> -
> >>>>> +     if (curseg->segno == NULL_SEGNO) {
> >>>>> +             err = -ENOSPC;
> >>>>> +             goto unlock;
> >>>>> +     }
> >>>>>         stat_inc_seg_type(sbi, curseg);
> >>>>>
> >>>>>         locate_dirty_segment(sbi, segno);
> >>>>> @@ -3102,6 +3106,7 @@ void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
> >>>>>
> >>>>>         mutex_unlock(&curseg->curseg_mutex);
> >>>>>         f2fs_up_read(&SM_I(sbi)->curseg_lock);
> >>>>> +     return err;
> >>>>>     }
> >>>>>
> >>>>>     static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,
diff mbox series

Patch

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4331012..39dda7d 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -3701,7 +3701,7 @@  void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi,
 void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
 void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
 void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
-void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
+int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
 					unsigned int start, unsigned int end);
 int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, bool force);
 int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index c60b747..7a458fa 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -2037,8 +2037,11 @@  static int free_segment_range(struct f2fs_sb_info *sbi,
 	mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
 
 	/* Move out cursegs from the target range */
-	for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++)
-		f2fs_allocate_segment_for_resize(sbi, type, start, end);
+	for (type = CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; type++) {
+		err = f2fs_allocate_segment_for_resize(sbi, type, start, end);
+		if (err)
+			goto out;
+	}
 
 	/* do GC to move out valid blocks in the range */
 	err = f2fs_gc_range(sbi, start, end, dry_run, 0);
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 1bb3019..2a07b9d 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3071,11 +3071,12 @@  static bool need_new_seg(struct f2fs_sb_info *sbi, int type)
 	return false;
 }
 
-void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
+int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
 					unsigned int start, unsigned int end)
 {
 	struct curseg_info *curseg = CURSEG_I(sbi, type);
 	unsigned int segno;
+	int err = 0;
 
 	f2fs_down_read(&SM_I(sbi)->curseg_lock);
 	mutex_lock(&curseg->curseg_mutex);
@@ -3089,7 +3090,10 @@  void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
 		change_curseg(sbi, type);
 	else
 		new_curseg(sbi, type, true);
-
+	if (curseg->segno == NULL_SEGNO) {
+		err = -ENOSPC;
+		goto unlock;
+	}
 	stat_inc_seg_type(sbi, curseg);
 
 	locate_dirty_segment(sbi, segno);
@@ -3102,6 +3106,7 @@  void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int type,
 
 	mutex_unlock(&curseg->curseg_mutex);
 	f2fs_up_read(&SM_I(sbi)->curseg_lock);
+	return err;
 }
 
 static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,