2023-06-28 13:31:23

by Zhihao Cheng

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

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 <[email protected]>
---
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



2023-06-28 14:55:55

by Jan Kara

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

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 <[email protected]>

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

Reviewed-by: Jan Kara <[email protected]>

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 <[email protected]>
SUSE Labs, CR