On Thu, Nov 21, 2019 at 01:30:36PM -0500, Theodore Ts'o wrote: > This allows us to test various error handling code paths > > Signed-off-by: Theodore Ts'o <tytso@xxxxxxx> > --- > fs/ext4/balloc.c | 4 +++- > fs/ext4/ext4.h | 42 ++++++++++++++++++++++++++++++++++++++++++ > fs/ext4/ialloc.c | 4 +++- > fs/ext4/inode.c | 6 +++++- > fs/ext4/namei.c | 11 ++++++++--- > fs/ext4/sysfs.c | 23 +++++++++++++++++++++++ > 6 files changed, 84 insertions(+), 6 deletions(-) > > diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c > index 102c38527a10..5f993a411251 100644 > --- a/fs/ext4/balloc.c > +++ b/fs/ext4/balloc.c > @@ -371,7 +371,8 @@ static int ext4_validate_block_bitmap(struct super_block *sb, > if (buffer_verified(bh)) > goto verified; > if (unlikely(!ext4_block_bitmap_csum_verify(sb, block_group, > - desc, bh))) { > + desc, bh) || > + ext4_simulate_fail(sb, EXT4_SIM_BBITMAP_CRC))) { > ext4_unlock_group(sb, block_group); > ext4_error(sb, "bg %u: bad block bitmap checksum", block_group); > ext4_mark_group_bitmap_corrupted(sb, block_group, > @@ -505,6 +506,7 @@ int ext4_wait_block_bitmap(struct super_block *sb, ext4_group_t block_group, > if (!desc) > return -EFSCORRUPTED; > wait_on_buffer(bh); > + ext4_simulate_fail_bh(sb, bh, EXT4_SIM_BBITMAP_EIO); > if (!buffer_uptodate(bh)) { > ext4_set_errno(sb, EIO); > ext4_error(sb, "Cannot read block bitmap - " > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 1c9ac0fc8715..e6798db4634c 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -1557,6 +1557,9 @@ struct ext4_sb_info { > /* Barrier between changing inodes' journal flags and writepages ops. */ > struct percpu_rw_semaphore s_journal_flag_rwsem; > struct dax_device *s_daxdev; > +#ifdef CONFIG_EXT4_DEBUG > + unsigned long s_simulate_fail; > +#endif > }; > > static inline struct ext4_sb_info *EXT4_SB(struct super_block *sb) > @@ -1575,6 +1578,45 @@ static inline int ext4_valid_inum(struct super_block *sb, unsigned long ino) > ino <= le32_to_cpu(EXT4_SB(sb)->s_es->s_inodes_count)); > } > > +static inline int ext4_simulate_fail(struct super_block *sb, > + unsigned long flag) Nit: bool? > +{ > +#ifdef CONFIG_EXT4_DEBUG > + unsigned long old, new; > + struct ext4_sb_info *sbi = EXT4_SB(sb); > + > + do { > + old = READ_ONCE(sbi->s_simulate_fail); > + if (likely((old & flag) == 0)) > + return 0; > + new = old & ~flag; > + } while (unlikely(cmpxchg(&sbi->s_simulate_fail, old, new) != old)); If I'm reading this correctly, this means that userspace sets a s_simulate_fail bit via sysfs knob, and the next time the filesystem calls ext4_simulate_fail with the same bit set in @flag we'll return true to say "simulate the failure" and clear the bit in s_simulate_fail? IOWs, the simulated failures have to be re-armed every time? Seems reasonable, but consider the possibility that in the future it might be useful if you could set up periodic failures (e.g. directory lookups fail 10% of the time) so that you can see how something like fsstress reacts to less-predictable failures? Of course that also increases the amount of fugly sysfs boilerplate so that each knob can have its own sysfs file... that alone is half of a reason not to do that. :( --D > + return 1; > +#else > + return 0; > +#endif > +} > + > +static inline void ext4_simulate_fail_bh(struct super_block *sb, > + struct buffer_head *bh, > + unsigned long flag) > +{ > + if (!IS_ERR(bh) && ext4_simulate_fail(sb, flag)) > + clear_buffer_uptodate(bh); > +} > + > +/* > + * Simulate_fail flags > + */ > +#define EXT4_SIM_BBITMAP_EIO 0x00000001 > +#define EXT4_SIM_BBITMAP_CRC 0x00000002 > +#define EXT4_SIM_IBITMAP_EIO 0x00000004 > +#define EXT4_SIM_IBITMAP_CRC 0x00000008 > +#define EXT4_SIM_INODE_EIO 0x00000010 > +#define EXT4_SIM_INODE_CRC 0x00000020 > +#define EXT4_SIM_DIRBLOCK_EIO 0x00000040 > +#define EXT4_SIM_DIRBLOCK_CRC 0x00000080 > + > /* > * Error number codes for s_{first,last}_error_errno > * > diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c > index 31a5fd6f5e6a..f7033594f4a0 100644 > --- a/fs/ext4/ialloc.c > +++ b/fs/ext4/ialloc.c > @@ -94,7 +94,8 @@ static int ext4_validate_inode_bitmap(struct super_block *sb, > goto verified; > blk = ext4_inode_bitmap(sb, desc); > if (!ext4_inode_bitmap_csum_verify(sb, block_group, desc, bh, > - EXT4_INODES_PER_GROUP(sb) / 8)) { > + EXT4_INODES_PER_GROUP(sb) / 8) || > + ext4_simulate_fail(sb, EXT4_SIM_IBITMAP_CRC)) { > ext4_unlock_group(sb, block_group); > ext4_error(sb, "Corrupt inode bitmap - block_group = %u, " > "inode_bitmap = %llu", block_group, blk); > @@ -192,6 +193,7 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group) > get_bh(bh); > submit_bh(REQ_OP_READ, REQ_META | REQ_PRIO, bh); > wait_on_buffer(bh); > + ext4_simulate_fail_bh(sb, bh, EXT4_SIM_IBITMAP_EIO); > if (!buffer_uptodate(bh)) { > put_bh(bh); > ext4_set_errno(sb, EIO); > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index b67ffa24ae0a..564f88040de9 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -4243,6 +4243,8 @@ static int __ext4_get_inode_loc(struct inode *inode, > bh = sb_getblk(sb, block); > if (unlikely(!bh)) > return -ENOMEM; > + if (ext4_simulate_fail(sb, EXT4_SIM_INODE_EIO)) > + goto simulate_eio; > if (!buffer_uptodate(bh)) { > lock_buffer(bh); > > @@ -4341,6 +4343,7 @@ static int __ext4_get_inode_loc(struct inode *inode, > blk_finish_plug(&plug); > wait_on_buffer(bh); > if (!buffer_uptodate(bh)) { > + simulate_eio: > ext4_set_errno(inode->i_sb, EIO); > EXT4_ERROR_INODE_BLOCK(inode, block, > "unable to read itable block"); > @@ -4555,7 +4558,8 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino, > sizeof(gen)); > } > > - if (!ext4_inode_csum_verify(inode, raw_inode, ei)) { > + if (!ext4_inode_csum_verify(inode, raw_inode, ei) || > + ext4_simulate_fail(sb, EXT4_SIM_INODE_CRC)) { > ext4_set_errno(inode->i_sb, EFSBADCRC); > ext4_error_inode(inode, function, line, 0, > "iget: checksum invalid"); > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index cee7c28e070d..be14e33e5103 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -109,7 +109,10 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode, > struct ext4_dir_entry *dirent; > int is_dx_block = 0; > > - bh = ext4_bread(NULL, inode, block, 0); > + if (ext4_simulate_fail(inode->i_sb, EXT4_SIM_DIRBLOCK_EIO)) > + bh = ERR_PTR(-EIO); > + else > + bh = ext4_bread(NULL, inode, block, 0); > if (IS_ERR(bh)) { > __ext4_warning(inode->i_sb, func, line, > "inode #%lu: lblock %lu: comm %s: " > @@ -153,7 +156,8 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode, > * caller is sure it should be an index block. > */ > if (is_dx_block && type == INDEX) { > - if (ext4_dx_csum_verify(inode, dirent)) > + if (ext4_dx_csum_verify(inode, dirent) && > + !ext4_simulate_fail(inode->i_sb, EXT4_SIM_DIRBLOCK_CRC)) > set_buffer_verified(bh); > else { > ext4_set_errno(inode->i_sb, EFSBADCRC); > @@ -164,7 +168,8 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode, > } > } > if (!is_dx_block) { > - if (ext4_dirblock_csum_verify(inode, bh)) > + if (ext4_dirblock_csum_verify(inode, bh) && > + !ext4_simulate_fail(inode->i_sb, EXT4_SIM_DIRBLOCK_CRC)) > set_buffer_verified(bh); > else { > ext4_set_errno(inode->i_sb, EFSBADCRC); > diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c > index eb1efad0e20a..a990d28d191b 100644 > --- a/fs/ext4/sysfs.c > +++ b/fs/ext4/sysfs.c > @@ -29,6 +29,7 @@ typedef enum { > attr_last_error_time, > attr_feature, > attr_pointer_ui, > + attr_pointer_ul, > attr_pointer_atomic, > attr_journal_task, > } attr_id_t; > @@ -160,6 +161,9 @@ static struct ext4_attr ext4_attr_##_name = { \ > #define EXT4_RW_ATTR_SBI_UI(_name,_elname) \ > EXT4_ATTR_OFFSET(_name, 0644, pointer_ui, ext4_sb_info, _elname) > > +#define EXT4_RW_ATTR_SBI_UL(_name,_elname) \ > + EXT4_ATTR_OFFSET(_name, 0644, pointer_ul, ext4_sb_info, _elname) > + > #define EXT4_ATTR_PTR(_name,_mode,_id,_ptr) \ > static struct ext4_attr ext4_attr_##_name = { \ > .attr = {.name = __stringify(_name), .mode = _mode }, \ > @@ -194,6 +198,9 @@ EXT4_RW_ATTR_SBI_UI(warning_ratelimit_interval_ms, s_warning_ratelimit_state.int > EXT4_RW_ATTR_SBI_UI(warning_ratelimit_burst, s_warning_ratelimit_state.burst); > EXT4_RW_ATTR_SBI_UI(msg_ratelimit_interval_ms, s_msg_ratelimit_state.interval); > EXT4_RW_ATTR_SBI_UI(msg_ratelimit_burst, s_msg_ratelimit_state.burst); > +#ifdef CONFIG_EXT4_DEBUG > +EXT4_RW_ATTR_SBI_UL(simulate_fail, s_simulate_fail); > +#endif > EXT4_RO_ATTR_ES_UI(errors_count, s_error_count); > EXT4_ATTR(first_error_time, 0444, first_error_time); > EXT4_ATTR(last_error_time, 0444, last_error_time); > @@ -228,6 +235,9 @@ static struct attribute *ext4_attrs[] = { > ATTR_LIST(first_error_time), > ATTR_LIST(last_error_time), > ATTR_LIST(journal_task), > +#ifdef CONFIG_EXT4_DEBUG > + ATTR_LIST(simulate_fail), > +#endif > NULL, > }; > ATTRIBUTE_GROUPS(ext4); > @@ -318,6 +328,11 @@ static ssize_t ext4_attr_show(struct kobject *kobj, > else > return snprintf(buf, PAGE_SIZE, "%u\n", > *((unsigned int *) ptr)); > + case attr_pointer_ul: > + if (!ptr) > + return 0; > + return snprintf(buf, PAGE_SIZE, "%lu\n", > + *((unsigned long *) ptr)); > case attr_pointer_atomic: > if (!ptr) > return 0; > @@ -361,6 +376,14 @@ static ssize_t ext4_attr_store(struct kobject *kobj, > else > *((unsigned int *) ptr) = t; > return len; > + case attr_pointer_ul: > + if (!ptr) > + return 0; > + ret = kstrtoul(skip_spaces(buf), 0, &t); > + if (ret) > + return ret; > + *((unsigned long *) ptr) = t; > + return len; > case attr_inode_readahead: > return inode_readahead_blks_store(sbi, buf, len); > case attr_trigger_test_error: > -- > 2.23.0 >