diff mbox series

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

Message ID cde56d55c6546a7861165acb8299e413d815e794.1688927487.git.sweettea-kernel@dorminy.me (mailing list archive)
State New, archived
Headers show
Series fscrypt: add extent encryption | expand

Commit Message

Sweet Tea Dorminy July 9, 2023, 6:53 p.m. UTC
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(-)

Comments

Josef Bacik July 17, 2023, 2:59 p.m. UTC | #1
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
diff mbox series

Patch

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;
 		}
 	}
+
 	/*
 	 * We return 0 if we successfully did something: removed a claim to the
 	 * key, wiped the secret, or tried locking the files again.  Users need