Re: [PATCH] ext4: Fix deadlock with fs freezing and EA inodes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Nov 27, 2020, at 4:06 AM, Jan Kara <jack@xxxxxxx> wrote:
> 
> Xattr code using inodes with large xattr data can end up dropping last
> inode reference (and thus deleting the inode) from places like
> ext4_xattr_set_entry(). That function is called with transaction started
> and so ext4_evict_inode() can deadlock against fs freezing like:
> 
> CPU1					CPU2
> 
> removexattr()				freeze_super()
>  vfs_removexattr()
>    ext4_xattr_set()
>      handle = ext4_journal_start()
>      ...
>      ext4_xattr_set_entry()
>        iput(old_ea_inode)
>          ext4_evict_inode(old_ea_inode)
> 					  sb->s_writers.frozen = SB_FREEZE_FS;
> 					  sb_wait_write(sb, SB_FREEZE_FS);
> 					  ext4_freeze()
> 					    jbd2_journal_lock_updates()
> 					      -> blocks waiting for all
> 					         handles to stop
>            sb_start_intwrite()
> 	      -> blocks as sb is already in SB_FREEZE_FS state
> 
> Generally it is advisable to delete inodes from a separate transaction
> as it can consume quite some credits however in this case it would be
> quite clumsy and furthermore the credits for inode deletion are quite
> limited and already accounted for. So just tweak ext4_evict_inode() to
> avoid freeze protection if we have transaction already started and thus
> it is not really needed anyway.
> 
> CC: stable@xxxxxxxxxxxxxxx
> Fixes: dec214d00e0d ("ext4: xattr inode deduplication")
> CC: Tahsin Erdogan <tahsin@xxxxxxxxxx>
> Signed-off-by: Jan Kara <jack@xxxxxxx>

Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx>

> ---
> fs/ext4/inode.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 72534319fae5..777eb08b29cd 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -175,6 +175,7 @@ void ext4_evict_inode(struct inode *inode)
> 	 */
> 	int extra_credits = 6;
> 	struct ext4_xattr_inode_array *ea_inode_array = NULL;
> +	bool freeze_protected = false;
> 
> 	trace_ext4_evict_inode(inode);
> 
> @@ -232,9 +233,14 @@ void ext4_evict_inode(struct inode *inode)
> 
> 	/*
> 	 * Protect us against freezing - iput() caller didn't have to have any
> -	 * protection against it
> +	 * protection against it. When we are in a running transaction though,
> +	 * we are already protected against freezing and we cannot grab further
> +	 * protection due to lock ordering constraints.
> 	 */
> -	sb_start_intwrite(inode->i_sb);
> +	if (!ext4_journal_current_handle()) {
> +		sb_start_intwrite(inode->i_sb);
> +		freeze_protected = true;
> +	}
> 
> 	if (!IS_NOQUOTA(inode))
> 		extra_credits += EXT4_MAXQUOTAS_DEL_BLOCKS(inode->i_sb);
> @@ -253,7 +259,8 @@ void ext4_evict_inode(struct inode *inode)
> 		 * cleaned up.
> 		 */
> 		ext4_orphan_del(NULL, inode);
> -		sb_end_intwrite(inode->i_sb);
> +		if (freeze_protected)
> +			sb_end_intwrite(inode->i_sb);
> 		goto no_delete;
> 	}
> 
> @@ -294,7 +301,8 @@ void ext4_evict_inode(struct inode *inode)
> stop_handle:
> 		ext4_journal_stop(handle);
> 		ext4_orphan_del(NULL, inode);
> -		sb_end_intwrite(inode->i_sb);
> +		if (freeze_protected)
> +			sb_end_intwrite(inode->i_sb);
> 		ext4_xattr_inode_array_free(ea_inode_array);
> 		goto no_delete;
> 	}
> @@ -323,7 +331,8 @@ void ext4_evict_inode(struct inode *inode)
> 	else
> 		ext4_free_inode(handle, inode);
> 	ext4_journal_stop(handle);
> -	sb_end_intwrite(inode->i_sb);
> +	if (freeze_protected)
> +		sb_end_intwrite(inode->i_sb);
> 	ext4_xattr_inode_array_free(ea_inode_array);
> 	return;
> no_delete:
> --
> 2.16.4
> 


Cheers, Andreas





Attachment: signature.asc
Description: Message signed with OpenPGP


[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux