Re: [PATCH 2/4] f2fs: handle dirty segments inside refresh_sit_entry

[Changman Lee <cm224.lee@samsung.com>]

Hi,
I found some redundant code in  your patch.
I think that locate_dirty_segment(sbi, old_cursegno) equals to
locate_dirty_segment(sbi, GET_SEGNO(sbi, new)) in refresh_sit_entry.
Because *new_blkaddr is a block belonging to old_cursegno.
How do you think?


On 화, 2014-01-28 at 14:54 +0900, Jaegeuk Kim wrote:
> This patch cleans up the refresh_sit_entry to handle locate_dirty_segments.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk.kim@samsung.com>
> ---
>  fs/f2fs/f2fs.h    |  1 +
>  fs/f2fs/segment.c | 19 ++++++++-----------
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 42903c3..6e9515d 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1132,6 +1132,7 @@ void destroy_node_manager_caches(void);
>  void f2fs_balance_fs(struct f2fs_sb_info *);
>  void f2fs_balance_fs_bg(struct f2fs_sb_info *);
>  void invalidate_blocks(struct f2fs_sb_info *, block_t);
> +void refresh_sit_entry(struct f2fs_sb_info *, block_t, block_t);
>  void clear_prefree_segments(struct f2fs_sb_info *);
>  int npages_for_summary_flush(struct f2fs_sb_info *);
>  void allocate_new_segments(struct f2fs_sb_info *);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 7caac5f..89aa503 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -434,12 +434,14 @@ static void update_sit_entry(struct f2fs_sb_info *sbi, block_t blkaddr, int del)
>  		get_sec_entry(sbi, segno)->valid_blocks += del;
>  }
>  
> -static void refresh_sit_entry(struct f2fs_sb_info *sbi,
> -			block_t old_blkaddr, block_t new_blkaddr)
> +void refresh_sit_entry(struct f2fs_sb_info *sbi, block_t old, block_t new)
>  {
> -	update_sit_entry(sbi, new_blkaddr, 1);
> -	if (GET_SEGNO(sbi, old_blkaddr) != NULL_SEGNO)
> -		update_sit_entry(sbi, old_blkaddr, -1);
> +	update_sit_entry(sbi, new, 1);
> +	if (GET_SEGNO(sbi, old) != NULL_SEGNO)
> +		update_sit_entry(sbi, old, -1);
> +
> +	locate_dirty_segment(sbi, GET_SEGNO(sbi, old));
> +	locate_dirty_segment(sbi, GET_SEGNO(sbi, new));
>  }
>  
>  void invalidate_blocks(struct f2fs_sb_info *sbi, block_t addr)
> @@ -886,12 +888,11 @@ void allocate_data_block(struct f2fs_sb_info *sbi, struct page *page,
>  	 * since SSR needs latest valid block information.
>  	 */
>  	refresh_sit_entry(sbi, old_blkaddr, *new_blkaddr);
> +	locate_dirty_segment(sbi, old_cursegno);
>  
>  	if (!__has_curseg_space(sbi, type))
>  		sit_i->s_ops->allocate_segment(sbi, type, false);
>  
> -	locate_dirty_segment(sbi, old_cursegno);
> -	locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr));
>  	mutex_unlock(&sit_i->sentry_lock);
>  
>  	if (page && IS_NODESEG(type))
> @@ -992,9 +993,7 @@ void recover_data_page(struct f2fs_sb_info *sbi,
>  	__add_sum_entry(sbi, type, sum);
>  
>  	refresh_sit_entry(sbi, old_blkaddr, new_blkaddr);
> -
>  	locate_dirty_segment(sbi, old_cursegno);
> -	locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr));
>  
>  	mutex_unlock(&sit_i->sentry_lock);
>  	mutex_unlock(&curseg->curseg_mutex);
> @@ -1045,9 +1044,7 @@ void rewrite_node_page(struct f2fs_sb_info *sbi,
>  	f2fs_submit_page_mbio(sbi, page, new_blkaddr, &fio);
>  	f2fs_submit_merged_bio(sbi, NODE, WRITE);
>  	refresh_sit_entry(sbi, old_blkaddr, new_blkaddr);
> -
>  	locate_dirty_segment(sbi, old_cursegno);
> -	locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr));
>  
>  	mutex_unlock(&sit_i->sentry_lock);
>  	mutex_unlock(&curseg->curseg_mutex);




------------------------------------------------------------------------------
Managing the Performance of Cloud-Based Applications
Take advantage of what the Cloud has to offer - Avoid Common Pitfalls.
Read the Whitepaper.
http://pubads.g.doubleclick.net/gampad/clk?id=121051231&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel