[2/2] btrfs: file_remove_privs needs an exclusive lock

Commit Message

Bernd Schubert Aug. 31, 2023, 11:24 a.m. UTC

file_remove_privs might call into notify_change(), which
requires to hold an exclusive lock.
In order to keep the shared lock for most IOs it now first
checks if privilege changes are needed, then switches to
the exclusive lock, rechecks and only then calls file_remove_privs.
This makes usage of the new exported function
file_needs_remove_privs().

The file_remove_privs code path is not optimized, under the
assumption that it would be a rare call (file_remove_privs
calls file_needs_remove_privs a 2nd time).

Fixes: e9adabb9712e ("btrfs: use shared lock for direct writes within EOF")
Cc: Christoph Hellwig <[e-mail]>
Cc: Goldwyn Rodrigues <[e-mail]>
Cc: Miklos Szeredi <[e-mail]>
Cc: Dharmendra Singh <[e-mail]>
Cc: Chris Mason <[e-mail]>
Cc: Josef Bacik <[e-mail]>
Cc: David Sterba <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Signed-off-by: Bernd Schubert <[e-mail]>
---
 fs/btrfs/file.c | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

Patch

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index fd03e689a6be..3162ec245d57 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1125,7 +1125,7 @@  static void update_time_for_write(struct inode *inode)
 }
 
 static int btrfs_write_check(struct kiocb *iocb, struct iov_iter *from,
-			     size_t count)
+			     size_t count, bool *shared_lock)
 {
 	struct file *file = iocb->ki_filp;
 	struct inode *inode = file_inode(file);
@@ -1145,9 +1145,17 @@  static int btrfs_write_check(struct kiocb *iocb, struct iov_iter *from,
 	    !(BTRFS_I(inode)->flags & (BTRFS_INODE_NODATACOW | BTRFS_INODE_PREALLOC)))
 		return -EAGAIN;
 
-	ret = file_remove_privs(file);
-	if (ret)
-		return ret;
+	ret = file_needs_remove_privs(file);
+	if (ret) {
+		if (shared_lock && *shared_lock) {
+			*shared_lock = false;
+			return -EAGAIN;
+		}
+
+		ret = file_remove_privs(file);
+		if (ret)
+			return ret;
+	}
 
 	/*
 	 * We reserve space for updating the inode when we reserve space for the
@@ -1204,7 +1212,7 @@  static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
 	if (ret <= 0)
 		goto out;
 
-	ret = btrfs_write_check(iocb, i, ret);
+	ret = btrfs_write_check(iocb, i, ret, NULL);
 	if (ret < 0)
 		goto out;
 
@@ -1462,13 +1470,20 @@  static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
 	ssize_t err;
 	unsigned int ilock_flags = 0;
 	struct iomap_dio *dio;
+	bool shared_lock;
 
 	if (iocb->ki_flags & IOCB_NOWAIT)
 		ilock_flags |= BTRFS_ILOCK_TRY;
 
-	/* If the write DIO is within EOF, use a shared lock */
-	if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode))
+	/* If the write DIO is within EOF, use a shared lock and also only
+	 * if security bits will likely not be dropped. Either will need
+	 * to be rechecked after the lock was acquired.
+	 */
+	if (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode) &&
+	    IS_NOSEC(inode)) {
 		ilock_flags |= BTRFS_ILOCK_SHARED;
+		shared_lock = true;
+	}
 
 relock:
 	err = btrfs_inode_lock(BTRFS_I(inode), ilock_flags);
@@ -1481,8 +1496,15 @@  static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
 		return err;
 	}
 
-	err = btrfs_write_check(iocb, from, err);
+	err = btrfs_write_check(iocb, from, err, &shared_lock);
 	if (err < 0) {
+		if (err == -EAGAIN && ilock_flags & BTRFS_ILOCK_SHARED &&
+		    !shared_lock) {
+			btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
+			ilock_flags &= ~BTRFS_ILOCK_SHARED;
+			goto relock;
+		}
+
 		btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
 		goto out;
 	}
@@ -1496,6 +1518,7 @@  static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
 	    pos + iov_iter_count(from) > i_size_read(inode)) {
 		btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
 		ilock_flags &= ~BTRFS_ILOCK_SHARED;
+		shared_lock = false;
 		goto relock;
 	}
 
@@ -1632,7 +1655,7 @@  static ssize_t btrfs_encoded_write(struct kiocb *iocb, struct iov_iter *from,
 	if (ret || encoded->len == 0)
 		goto out;
 
-	ret = btrfs_write_check(iocb, from, encoded->len);
+	ret = btrfs_write_check(iocb, from, encoded->len, NULL);
 	if (ret < 0)
 		goto out;