Re: [PATCH] ext4: Fix unttached inode after power cut with orphan file feature enabled

[Jan Kara <jack@suse.cz>]

On Wed 28-06-23 21:20:11, Zhihao Cheng wrote:
> Running generic/475(filesystem consistent tests after power cut) could
> easily trigger unattached inode error while doing fsck:
>   Unattached zero-length inode 39405.  Clear? no
> 
>   Unattached inode 39405
>   Connect to /lost+found? no
> 
> Above inconsistence is caused by following process:
>        P1                       P2
> ext4_create
>  inode = ext4_new_inode_start_handle  // itable records nlink=1
>  ext4_add_nondir
>    err = ext4_add_entry  // ENOSPC
>     ext4_append
>      ext4_bread
>       ext4_getblk
>        ext4_map_blocks // returns ENOSPC
>    drop_nlink(inode) // won't be updated into disk inode
>    ext4_orphan_add(handle, inode)
>     ext4_orphan_file_add
>  ext4_journal_stop(handle)
> 		      jbd2_journal_commit_transaction // commit success
>               >> power cut <<
> ext4_fill_super
>  ext4_load_and_init_journal   // itable records nlink=1
>  ext4_orphan_cleanup
>   ext4_process_orphan
>    if (inode->i_nlink)        // true, inode won't be deleted
> 
> Then, allocated inode will be reserved on disk and corresponds to no
> dentries, so e2fsck reports 'unattached inode' problem.
> 
> The problem won't happen if orphan file feature is disabled, because
> ext4_orphan_add() will update disk inode in orphan list mode. There
> are several places not updating disk inode while putting inode into
> orphan area, such as ext4_add_nondir(), ext4_symlink() and whiteout
> in ext4_rename(). Fix it by updating inode into disk in all error
> branches of these places.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217605
> Fixes: 02f310fcf47f ("ext4: Speedup ext4 orphan inode handling")
> Signed-off-by: Zhihao Cheng <chengzhihao1@huawei.com>

Nice catch. Thanks for fixing this. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/namei.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 0caf6c730ce3..6bcc3770ee19 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -2799,6 +2799,7 @@ static int ext4_add_nondir(handle_t *handle,
>  		return err;
>  	}
>  	drop_nlink(inode);
> +	ext4_mark_inode_dirty(handle, inode);
>  	ext4_orphan_add(handle, inode);
>  	unlock_new_inode(inode);
>  	return err;
> @@ -3436,6 +3437,7 @@ static int ext4_symlink(struct mnt_idmap *idmap, struct inode *dir,
>  
>  err_drop_inode:
>  	clear_nlink(inode);
> +	ext4_mark_inode_dirty(handle, inode);
>  	ext4_orphan_add(handle, inode);
>  	unlock_new_inode(inode);
>  	if (handle)
> @@ -4021,6 +4023,7 @@ static int ext4_rename(struct mnt_idmap *idmap, struct inode *old_dir,
>  			ext4_resetent(handle, &old,
>  				      old.inode->i_ino, old_file_type);
>  			drop_nlink(whiteout);
> +			ext4_mark_inode_dirty(handle, whiteout);
>  			ext4_orphan_add(handle, whiteout);
>  		}
>  		unlock_new_inode(whiteout);
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR