linux-kernel - Re: [PATCH 8/9] f2fs: avoid race for summary information

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 3 Apr 2013 14:54:02 +0900
From:	Namjae Jeon <linkinjeon@...il.com>
To:	Jaegeuk Kim <jaegeuk.kim@...sung.com>
Cc:	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [PATCH 8/9] f2fs: avoid race for summary information

2013/4/1, Jaegeuk Kim <jaegeuk.kim@...sung.com>:
> In order to do GC more reliably, I'd like to lock the vicitm summary page
> until its GC is completed, and also prevent any checkpoint process.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk.kim@...sung.com>
> ---
>  fs/f2fs/gc.c    | 8 +-------
>  fs/f2fs/node.c  | 2 +-
>  fs/f2fs/super.c | 7 +++++--
>  3 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 54aceb2..54ac13d 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -641,12 +641,6 @@ static void do_garbage_collect(struct f2fs_sb_info
> *sbi, unsigned int segno,
>  	if (IS_ERR(sum_page))
>  		return;
>
> -	/*
> -	 * CP needs to lock sum_page. In this time, we don't need
> -	 * to lock this page, because this summary page is not gone anywhere.
> -	 * Also, this page is not gonna be updated before GC is done.
> -	 */
> -	unlock_page(sum_page);
>  	sum = page_address(sum_page);
>
>  	switch (GET_SUM_TYPE((&sum->footer))) {
> @@ -660,7 +654,7 @@ static void do_garbage_collect(struct f2fs_sb_info *sbi,
> unsigned int segno,
>  	stat_inc_seg_count(sbi, GET_SUM_TYPE((&sum->footer)));
>  	stat_inc_call_count(sbi->stat_info);
>
> -	f2fs_put_page(sum_page, 0);
> +	f2fs_put_page(sum_page, 1);
>  }
>
>  int f2fs_gc(struct f2fs_sb_info *sbi)
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index c5360b7..7555fb7 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -1148,7 +1148,7 @@ static int f2fs_write_node_pages(struct address_space
> *mapping,
>
>  	/* First check balancing cached NAT entries */
>  	if (try_to_free_nats(sbi, NAT_ENTRY_PER_BLOCK)) {
> -		write_checkpoint(sbi, false);
> +		f2fs_sync_fs(sbi->sb, true);
>  		return 0;
>  	}
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index ea325c8..161e6c6 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -137,10 +137,13 @@ int f2fs_sync_fs(struct super_block *sb, int sync)
>  	if (!sbi->s_dirty && !get_pages(sbi, F2FS_DIRTY_NODES))
>  		return 0;
>
> -	if (sync)
> +	if (sync) {
> +		mutex_lock(&sbi->gc_mutex);
>  		write_checkpoint(sbi, false);
> -	else
> +		mutex_unlock(&sbi->gc_mutex);
> +	} else {
>  		f2fs_balance_fs(sbi);
> +	}
We can remove { } of else because single line code is used.
Reviewed-by: Namjae Jeon <namjae.jeon@...sung.com>

Thanks.
>
>  	return 0;
>  }
> --
> 1.8.1.3.566.gaa39828
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/