Re: [PATCH v2 09/14] fscrypt: move function call warning of busy inodes

[Josef Bacik <josef@toxicpanda.com>]

On Sun, Jul 09, 2023 at 02:53:42PM -0400, Sweet Tea Dorminy wrote:
> Extent encryption will want to attempt to evict inodes, and not warn of
> busy ones, before removing the key instead of after as it is at present.
> Therefore pull the call for check_for_busy_inodes() out of
> try_to_lock_encrypted_files() into its only callsite.
> 
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/keyring.c | 26 +++++++++++++-------------
>  1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
> index bfcd2ecbe481..c4499248b6cc 100644
> --- a/fs/crypto/keyring.c
> +++ b/fs/crypto/keyring.c
> @@ -947,8 +947,7 @@ static int check_for_busy_inodes(struct super_block *sb,
>  static int try_to_lock_encrypted_files(struct super_block *sb,
>  				       struct fscrypt_master_key *mk)
>  {
> -	int err1;
> -	int err2;
> +	int err;
>  
>  	/*
>  	 * An inode can't be evicted while it is dirty or has dirty pages.
> @@ -960,7 +959,7 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
>  	 * already call sync_filesystem() via sys_syncfs() or sys_sync().
>  	 */
>  	down_read(&sb->s_umount);
> -	err1 = sync_filesystem(sb);
> +	err = sync_filesystem(sb);
>  	up_read(&sb->s_umount);
>  	/* If a sync error occurs, still try to evict as much as possible. */
>  
> @@ -972,16 +971,7 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
>  	 */
>  	evict_dentries_for_decrypted_inodes(mk);
>  
> -	/*
> -	 * evict_dentries_for_decrypted_inodes() already iput() each inode in
> -	 * the list; any inodes for which that dropped the last reference will
> -	 * have been evicted due to fscrypt_drop_inode() detecting the key
> -	 * removal and telling the VFS to evict the inode.  So to finish, we
> -	 * just need to check whether any inodes couldn't be evicted.
> -	 */
> -	err2 = check_for_busy_inodes(sb, mk);
> -
> -	return err1 ?: err2;
> +	return err;
>  }
>  
>  /*
> @@ -1073,14 +1063,24 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
>  	up_write(&mk->mk_sem);
>  
>  	if (inodes_remain) {
> +		int err2;
>  		/* Some inodes still reference this key; try to evict them. */
>  		err = try_to_lock_encrypted_files(sb, mk);
> +		/* We already tried to iput() each inode referencing this key
> +		 * which would cause the inode to be evicted if that was the
> +		 * last reference (since fscrypt_drop_inode() would see the
> +		 * key removal). So the only remaining inodes referencing this
> +		 * key are still busy and couldn't be evicted; check for them.
> +		 */
> +		err2 = check_for_busy_inodes(sb, mk);
> +		err = err ?: err2;
>  		if (err == -EBUSY) {
>  			status_flags |=
>  				FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY;
>  			err = 0;
>  		}
>  	}
> +

Extra whitespace.  Thanks,

Josef