diff mbox series

ext4: Fix unttached inode after power cut with orphan file feature enabled

Message ID 20230628132011.650383-1-chengzhihao1@huawei.com
State New
Headers
Series ext4: Fix unttached inode after power cut with orphan file feature enabled |

Commit Message

Zhihao Cheng June 28, 2023, 1:20 p.m. UTC 
  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>
---
 fs/ext4/namei.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jan Kara June 28, 2023, 2:48 p.m. UTC | #1 
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
>
diff mbox series

Patch

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);