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;
Date:   Mon, 11 Apr 2022 14:20:41 -0700
From:   Jaegeuk Kim <jaegeuk@kernel.org>
To:     Chao Yu <chao@kernel.org>
Cc:     Yufen Yu <yuyufen@huawei.com>, linux-kernel@vger.kernel.org,
        linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 4/5] f2fs: get rid of stale fault injection
 code
Message-ID: <YlSbqay8y6oaCxR6@google.com>
References: <20220401071909.505086-1-yuyufen@huawei.com>
 <20220401071909.505086-5-yuyufen@huawei.com>
 <0c134e0e-b2d0-0bc0-42fc-cd220ff77e72@kernel.org>
 <25a24259-3ac3-81ab-1c28-f2a4886888b5@huawei.com>
 <778b1f04-a71b-a226-f7a7-4833fc9bf7c2@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <778b1f04-a71b-a226-f7a7-4833fc9bf7c2@kernel.org>
Precedence: bulk

On 04/11, Chao Yu wrote:
> On 2022/4/6 11:01, Yufen Yu via Linux-f2fs-devel wrote:
> > Hi,
> > 
> > On 2022/4/1 16:28, Chao Yu wrote:
> > > On 2022/4/1 15:19, Yufen Yu via Linux-f2fs-devel wrote:
> > > > Nowly, we can use new fault injection framework. Just delete the
> > > > stale fault injection code.
> > > > 
> > > > Signed-off-by: Yufen Yu <yuyufen@huawei.com>
> > > > ---
> > > > ? fs/f2fs/checkpoint.c |? 2 +-
> > > > ? fs/f2fs/f2fs.h?????? | 51 ++----------------------------------------
> > > > ? fs/f2fs/super.c????? | 53 --------------------------------------------
> > > > ? fs/f2fs/sysfs.c????? | 23 -------------------
> > > > ? 4 files changed, 3 insertions(+), 126 deletions(-)
> > > > 
> > 
> > ...
> > 
> > > > ????????????? break;
> > > > @@ -1963,14 +1920,6 @@ static int f2fs_show_options(struct seq_file *seq, struct dentry *root)
> > > > ????? if (F2FS_IO_SIZE_BITS(sbi))
> > > > ????????? seq_printf(seq, ",io_bits=%u",
> > > > ????????????????? F2FS_OPTION(sbi).write_io_size_bits);
> > > > -#ifdef CONFIG_F2FS_FAULT_INJECTION
> > > > -??? if (test_opt(sbi, FAULT_INJECTION)) {
> > > > -??????? seq_printf(seq, ",fault_injection=%u",
> > > > -??????????????? F2FS_OPTION(sbi).fault_info.inject_rate);
> > > > -??????? seq_printf(seq, ",fault_type=%u",
> > > > -??????????????? F2FS_OPTION(sbi).fault_info.inject_type);
> > > > -??? }
> > > > -#endif
> > > 
> > > This will cause regression due to it breaks application usage w/ -o
> > > fault_* mountoption..., I don't think this is the right way.
> > 
> > 
> > Thanks for catching this. I admit it's a problem. But, IMO fault_* mount
> > option are mostly been used in test, not in actual product. So, I think
> > it may just affect some test applications. With the common fault injection
> > framework, it can be more easy and flexible to do fault injection test.
> > Therefore, I want to remove the two mount options directly.
> > 
> > If you really worried about compatibility, how about just reserving the
> > two inject_* options but without doing any thing for them. We actually
> > configure fault injections by debugfs in this patch.
> > 
> > Or do you have more better suggestion?
> 
> Could you please consider to keep original logic of f2fs fault injection
> if user use inject_* options, otherwise following common fault injection
> framework?
> 
> Thoughts?

I think it'd be useful to test roll-forward recovery flow by using those mount
options, since runtime fault injection cannot enable it during mount.

BTW, what is the real benefit to use the fault injection framework?

> 
> Thanks,
> 
> > 
> > Thanks,
> > Yufen
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > 
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel