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]
Message-ID: <0c134e0e-b2d0-0bc0-42fc-cd220ff77e72@kernel.org>
Date:   Fri, 1 Apr 2022 16:28:52 +0800
From:   Chao Yu <chao@...nel.org>
To:     Yufen Yu <yuyufen@...wei.com>, jaegeuk@...nel.org
Cc:     linux-kernel@...r.kernel.org,
        linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH 4/5] f2fs: get rid of stale fault injection
 code

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@...wei.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(-)
> 
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 820eb95b08f1..88e419db5782 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -28,7 +28,7 @@ struct kmem_cache *f2fs_inode_entry_slab;
>   
>   void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io)
>   {
> -	f2fs_build_fault_attr(sbi, 0, 0);
> +	f2fs_stop_fault_inject(sbi);
>   	set_ckpt_flags(sbi, CP_ERROR_FLAG);
>   	if (!end_io)
>   		f2fs_flush_merged_writes(sbi);
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index beb935f807d6..61b07eb4bb7b 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -65,19 +65,6 @@ enum {
>   
>   extern struct dentry *f2fs_debugfs_root;
>   
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
> -#define F2FS_ALL_FAULT_TYPE		((1 << FAULT_MAX) - 1)
> -
> -struct f2fs_fault_info {
> -	atomic_t inject_ops;
> -	unsigned int inject_rate;
> -	unsigned int inject_type;
> -};
> -
> -extern const char *f2fs_fault_name[FAULT_MAX];
> -#define IS_FAULT_SET(fi, type) ((fi)->inject_type & (1 << (type)))
> -#endif
> -
>   /*
>    * For mount options
>    */
> @@ -148,9 +135,6 @@ struct f2fs_mount_info {
>   	kgid_t s_resgid;		/* reserved blocks for gid */
>   	int active_logs;		/* # of active logs */
>   	int inline_xattr_size;		/* inline xattr size */
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
> -	struct f2fs_fault_info fault_info;	/* For fault injection */
> -#endif
>   #ifdef CONFIG_QUOTA
>   	/* Names of quota files with journalled quota */
>   	char *s_qf_names[MAXQUOTAS];
> @@ -1860,42 +1844,18 @@ struct f2fs_sb_info {
>   };
>   
>   #ifdef CONFIG_F2FS_FAULT_INJECTION
> +extern const char *f2fs_fault_name[FAULT_MAX];
>   #define f2fs_show_injection_info(sbi, type)					\
>   	printk_ratelimited("%sF2FS-fs (%s) : inject %s in %s of %pS\n",	\
>   		KERN_INFO, sbi->sb->s_id,				\
>   		f2fs_fault_name[type],					\
>   		__func__, __builtin_return_address(0))
> -static inline bool time_to_inject(struct f2fs_sb_info *sbi, int type)
> -{
> -	struct f2fs_fault_info *ffi = &F2FS_OPTION(sbi).fault_info;
> -
> -	if (!ffi->inject_rate)
> -		return false;
> -
> -	if (!IS_FAULT_SET(ffi, type))
> -		return false;
> -
> -	atomic_inc(&ffi->inject_ops);
> -	if (atomic_read(&ffi->inject_ops) >= ffi->inject_rate) {
> -		atomic_set(&ffi->inject_ops, 0);
> -		return true;
> -	}
> -	return false;
> -}
> -#else
> -#define f2fs_show_injection_info(sbi, type) do { } while (0)
> -static inline bool time_to_inject(struct f2fs_sb_info *sbi, int type)
> -{
> -	return false;
> -}
> -#endif
> -
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
>   bool f2fs_should_fail(struct f2fs_sb_info *sbi, unsigned int type);
>   void f2fs_fault_inject_init(struct f2fs_sb_info *sbi);
>   void f2fs_fault_inject_fini(struct f2fs_sb_info *sbi);
>   void f2fs_stop_fault_inject(struct f2fs_sb_info *sbi);
>   #else
> +#define f2fs_show_injection_info(sbi, type) do { } while (0)
>   static inline bool f2fs_should_fail(struct f2fs_sb_info *sbi, unsigned int type)
>   {
>   	return false;
> @@ -4538,13 +4498,6 @@ static inline bool f2fs_need_verity(const struct inode *inode, pgoff_t idx)
>   	       idx < DIV_ROUND_UP(inode->i_size, PAGE_SIZE);
>   }
>   
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
> -extern void f2fs_build_fault_attr(struct f2fs_sb_info *sbi, unsigned int rate,
> -							unsigned int type);
> -#else
> -#define f2fs_build_fault_attr(sbi, rate, type)		do { } while (0)
> -#endif
> -
>   static inline bool is_journalled_quota(struct f2fs_sb_info *sbi)
>   {
>   #ifdef CONFIG_QUOTA
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index e18f60165f44..859ae5068761 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -64,22 +64,6 @@ const char *f2fs_fault_name[FAULT_MAX] = {
>   	[FAULT_LOCK_OP]		= "lock_op",
>   };
>   
> -void f2fs_build_fault_attr(struct f2fs_sb_info *sbi, unsigned int rate,
> -							unsigned int type)
> -{
> -	struct f2fs_fault_info *ffi = &F2FS_OPTION(sbi).fault_info;
> -
> -	if (rate) {
> -		atomic_set(&ffi->inject_ops, 0);
> -		ffi->inject_rate = rate;
> -	}
> -
> -	if (type)
> -		ffi->inject_type = type;
> -
> -	if (!rate && !type)
> -		memset(ffi, 0, sizeof(struct f2fs_fault_info));
> -}
>   #endif
>   
>   /* f2fs-wide shrinker description */
> @@ -122,8 +106,6 @@ enum {
>   	Opt_resuid,
>   	Opt_mode,
>   	Opt_io_size_bits,
> -	Opt_fault_injection,
> -	Opt_fault_type,
>   	Opt_lazytime,
>   	Opt_nolazytime,
>   	Opt_quota,
> @@ -198,8 +180,6 @@ static match_table_t f2fs_tokens = {
>   	{Opt_resuid, "resuid=%u"},
>   	{Opt_mode, "mode=%s"},
>   	{Opt_io_size_bits, "io_bits=%u"},
> -	{Opt_fault_injection, "fault_injection=%u"},
> -	{Opt_fault_type, "fault_type=%u"},
>   	{Opt_lazytime, "lazytime"},
>   	{Opt_nolazytime, "nolazytime"},
>   	{Opt_quota, "quota"},
> @@ -874,29 +854,6 @@ static int parse_options(struct super_block *sb, char *options, bool is_remount)
>   			}
>   			F2FS_OPTION(sbi).write_io_size_bits = arg;
>   			break;
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
> -		case Opt_fault_injection:
> -			if (args->from && match_int(args, &arg))
> -				return -EINVAL;
> -			f2fs_build_fault_attr(sbi, arg, F2FS_ALL_FAULT_TYPE);
> -			set_opt(sbi, FAULT_INJECTION);
> -			break;
> -
> -		case Opt_fault_type:
> -			if (args->from && match_int(args, &arg))
> -				return -EINVAL;
> -			f2fs_build_fault_attr(sbi, 0, arg);
> -			set_opt(sbi, FAULT_INJECTION);
> -			break;
> -#else
> -		case Opt_fault_injection:
> -			f2fs_info(sbi, "fault_injection options not supported");
> -			break;
> -
> -		case Opt_fault_type:
> -			f2fs_info(sbi, "fault_type options not supported");
> -			break;
> -#endif
>   		case Opt_lazytime:
>   			sb->s_flags |= SB_LAZYTIME;
>   			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,

>   #ifdef CONFIG_QUOTA
>   	if (test_opt(sbi, QUOTA))
>   		seq_puts(seq, ",quota");
> @@ -2076,8 +2025,6 @@ static void default_options(struct f2fs_sb_info *sbi)
>   #ifdef CONFIG_F2FS_FS_POSIX_ACL
>   	set_opt(sbi, POSIX_ACL);
>   #endif
> -
> -	f2fs_build_fault_attr(sbi, 0, 0);
>   }
>   
>   #ifdef CONFIG_QUOTA
> diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
> index 4c50aedd5144..85af2e97d565 100644
> --- a/fs/f2fs/sysfs.c
> +++ b/fs/f2fs/sysfs.c
> @@ -31,10 +31,6 @@ enum {
>   	F2FS_SBI,	/* struct f2fs_sb_info */
>   #ifdef CONFIG_F2FS_STAT_FS
>   	STAT_INFO,	/* struct f2fs_stat_info */
> -#endif
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
> -	FAULT_INFO_RATE,	/* struct f2fs_fault_info */
> -	FAULT_INFO_TYPE,	/* struct f2fs_fault_info */
>   #endif
>   	RESERVED_BLOCKS,	/* struct f2fs_sb_info */
>   	CPRC_INFO,	/* struct ckpt_req_control */
> @@ -76,11 +72,6 @@ static unsigned char *__struct_ptr(struct f2fs_sb_info *sbi, int struct_type)
>   		return (unsigned char *)NM_I(sbi);
>   	else if (struct_type == F2FS_SBI || struct_type == RESERVED_BLOCKS)
>   		return (unsigned char *)sbi;
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
> -	else if (struct_type == FAULT_INFO_RATE ||
> -					struct_type == FAULT_INFO_TYPE)
> -		return (unsigned char *)&F2FS_OPTION(sbi).fault_info;
> -#endif
>   #ifdef CONFIG_F2FS_STAT_FS
>   	else if (struct_type == STAT_INFO)
>   		return (unsigned char *)F2FS_STAT(sbi);
> @@ -429,12 +420,6 @@ static ssize_t __sbi_store(struct f2fs_attr *a,
>   	ret = kstrtoul(skip_spaces(buf), 0, &t);
>   	if (ret < 0)
>   		return ret;
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
> -	if (a->struct_type == FAULT_INFO_TYPE && t >= (1 << FAULT_MAX))
> -		return -EINVAL;
> -	if (a->struct_type == FAULT_INFO_RATE && t >= UINT_MAX)
> -		return -EINVAL;
> -#endif
>   	if (a->struct_type == RESERVED_BLOCKS) {
>   		spin_lock(&sbi->stat_lock);
>   		if (t > (unsigned long)(sbi->user_block_count -
> @@ -773,10 +758,6 @@ F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, readdir_ra, readdir_ra);
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, max_io_bytes, max_io_bytes);
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_pin_file_thresh, gc_pin_file_threshold);
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_super_block, extension_list, extension_list);
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
> -F2FS_RW_ATTR(FAULT_INFO_RATE, f2fs_fault_info, inject_rate, inject_rate);
> -F2FS_RW_ATTR(FAULT_INFO_TYPE, f2fs_fault_info, inject_type, inject_type);
> -#endif
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, data_io_flag, data_io_flag);
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, node_io_flag, node_io_flag);
>   F2FS_RW_ATTR(F2FS_SBI, f2fs_sb_info, gc_urgent_high_remaining, gc_urgent_high_remaining);
> @@ -892,10 +873,6 @@ static struct attribute *f2fs_attrs[] = {
>   	ATTR_LIST(max_io_bytes),
>   	ATTR_LIST(gc_pin_file_thresh),
>   	ATTR_LIST(extension_list),
> -#ifdef CONFIG_F2FS_FAULT_INJECTION
> -	ATTR_LIST(inject_rate),
> -	ATTR_LIST(inject_type),
> -#endif
>   	ATTR_LIST(data_io_flag),
>   	ATTR_LIST(node_io_flag),
>   	ATTR_LIST(gc_urgent_high_remaining),

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ