Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <25a24259-3ac3-81ab-1c28-f2a4886888b5@huawei.com>
Date:   Wed, 6 Apr 2022 11:01:23 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.7.0
Subject: Re: [f2fs-dev] [PATCH 4/5] f2fs: get rid of stale fault injection
 code
Content-Language: en-US
To:     Chao Yu <chao@kernel.org>, <jaegeuk@kernel.org>
CC:     <linux-kernel@vger.kernel.org>,
        <linux-f2fs-devel@lists.sourceforge.net>
References: <20220401071909.505086-1-yuyufen@huawei.com>
 <20220401071909.505086-5-yuyufen@huawei.com>
 <0c134e0e-b2d0-0bc0-42fc-cd220ff77e72@kernel.org>
From:   Yufen Yu <yuyufen@huawei.com>
In-Reply-To: <0c134e0e-b2d0-0bc0-42fc-cd220ff77e72@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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?

Thanks,
Yufen