Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <2a5564bb-463e-d163-0042-34fad42e2e46@kernel.org>
Date:   Thu, 10 Mar 2022 09:53:12 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.6.2
Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: evict inode cache for frozen fs
Content-Language: en-US
To:     Jaegeuk Kim <jaegeuk@kernel.org>, linux-kernel@vger.kernel.org,
        linux-f2fs-devel@lists.sourceforge.net
References: <20220309214834.3408741-1-jaegeuk@kernel.org>
From:   Chao Yu <chao@kernel.org>
In-Reply-To: <20220309214834.3408741-1-jaegeuk@kernel.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022/3/10 5:48, Jaegeuk Kim wrote:
> Let's purge inode cache in order to avoid the below deadlock.
> 
> [freeze test]                         shrinkder
> freeze_super
>   - pwercpu_down_write(SB_FREEZE_FS)
>                                         - super_cache_scan
>                                           - down_read(&sb->s_umount)
>                                             - prune_icache_sb
>                                              - dispose_list
>                                               - evict
>                                                - f2fs_evict_inode
> thaw_super
>   - down_write(&sb->s_umount);
>                                                - __percpu_down_read(SB_FREEZE_FS)

Ah, finally we catch this. :)

> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/debug.c | 1 +
>   fs/f2fs/f2fs.h  | 1 +
>   fs/f2fs/inode.c | 6 ++++--
>   fs/f2fs/super.c | 4 ++++
>   4 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index 9a13902c7702..cba5eab24595 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -338,6 +338,7 @@ static char *s_flag[] = {
>   	[SBI_QUOTA_SKIP_FLUSH]	= " quota_skip_flush",
>   	[SBI_QUOTA_NEED_REPAIR]	= " quota_need_repair",
>   	[SBI_IS_RESIZEFS]	= " resizefs",
> +	[SBI_IS_FREEZING]	= " freezefs",

Could you please update description of "sb_status" entry in
Documentation/ABI/testing/sysfs-fs-f2fs as well?

Thanks,

>   };
>   
>   static int stat_show(struct seq_file *s, void *v)
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 68d791ec8b27..da729f53daa8 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1293,6 +1293,7 @@ enum {
>   	SBI_QUOTA_SKIP_FLUSH,			/* skip flushing quota in current CP */
>   	SBI_QUOTA_NEED_REPAIR,			/* quota file may be corrupted */
>   	SBI_IS_RESIZEFS,			/* resizefs is in process */
> +	SBI_IS_FREEZING,			/* freezefs is in process */
>   };
>   
>   enum {
> diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c
> index ab8e0c06c78c..71f232dcf3c2 100644
> --- a/fs/f2fs/inode.c
> +++ b/fs/f2fs/inode.c
> @@ -778,7 +778,8 @@ void f2fs_evict_inode(struct inode *inode)
>   	f2fs_remove_ino_entry(sbi, inode->i_ino, UPDATE_INO);
>   	f2fs_remove_ino_entry(sbi, inode->i_ino, FLUSH_INO);
>   
> -	sb_start_intwrite(inode->i_sb);
> +	if (!is_sbi_flag_set(sbi, SBI_IS_FREEZING))
> +		sb_start_intwrite(inode->i_sb);
>   	set_inode_flag(inode, FI_NO_ALLOC);
>   	i_size_write(inode, 0);
>   retry:
> @@ -809,7 +810,8 @@ void f2fs_evict_inode(struct inode *inode)
>   		if (dquot_initialize_needed(inode))
>   			set_sbi_flag(sbi, SBI_QUOTA_NEED_REPAIR);
>   	}
> -	sb_end_intwrite(inode->i_sb);
> +	if (!is_sbi_flag_set(sbi, SBI_IS_FREEZING))
> +		sb_end_intwrite(inode->i_sb);
>   no_delete:
>   	dquot_drop(inode);
>   
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index 8e3840973077..4b570b5c2674 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1663,11 +1663,15 @@ static int f2fs_freeze(struct super_block *sb)
>   	/* ensure no checkpoint required */
>   	if (!llist_empty(&F2FS_SB(sb)->cprc_info.issue_list))
>   		return -EINVAL;
> +
> +	/* to avoid deadlock on f2fs_evict_inode->SB_FREEZE_FS */
> +	set_sbi_flag(F2FS_SB(sb), SBI_IS_FREEZING);
>   	return 0;
>   }
>   
>   static int f2fs_unfreeze(struct super_block *sb)
>   {
> +	clear_sbi_flag(F2FS_SB(sb), SBI_IS_FREEZING);
>   	return 0;
>   }
>