public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Bernd Schubert <bschubert@ddn.com>
To: linux-fsdevel@vger.kernel.org
Cc: bernd.schubert@fastmail.fm, miklos@szeredi.hu, dsingh@ddn.com,
	Bernd Schubert <bschubert@ddn.com>,
	Goldwyn Rodrigues <rgoldwyn@suse.com>, Chris Mason <clm@fb.com>,
	Josef Bacik <josef@toxicpanda.com>,
	David Sterba <dsterba@suse.com>,
	linux-btrfs@vger.kernel.org
Subject: [PATCH 2/2] btrfs: file_remove_privs needs an exclusive lock
Date: Wed, 30 Aug 2023 20:15:19 +0200	[thread overview]
Message-ID: <20230830181519.2964941-3-bschubert@ddn.com> (raw)
In-Reply-To: <20230830181519.2964941-1-bschubert@ddn.com>

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: Goldwyn Rodrigues <rgoldwyn@suse.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Dharmendra Singh <dsingh@ddn.com>
Cc: Chris Mason <clm@fb.com>
Cc: Josef Bacik <josef@toxicpanda.com>
Cc: David Sterba <dsterba@suse.com>
Cc: linux-btrfs@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
 fs/btrfs/file.c | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index fd03e689a6be..89c869ab131d 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, unsigned int *ilock_flags)
 {
 	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 (ilock_flags && *ilock_flags & BTRFS_ILOCK_SHARED) {
+			*ilock_flags &= ~BTRFS_ILOCK_SHARED;
+			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,16 @@ 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 has_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 (iocb->ki_pos + iov_iter_count(from) <= i_size_read(inode)) {
 		ilock_flags |= BTRFS_ILOCK_SHARED;
+		has_shared_lock = true;
+	}
 
 relock:
 	err = btrfs_inode_lock(BTRFS_I(inode), ilock_flags);
@@ -1481,8 +1492,17 @@ static ssize_t btrfs_direct_write(struct kiocb *iocb, struct iov_iter *from)
 		return err;
 	}
 
-	err = btrfs_write_check(iocb, from, err);
+	/* might uset BTRFS_ILOCK_SHARED */
+	err = btrfs_write_check(iocb, from, err, &ilock_flags);
 	if (err < 0) {
+		if (err == -EAGAIN && has_shared_lock &&
+		    !(ilock_flags & BTRFS_ILOCK_SHARED)) {
+			btrfs_inode_unlock(BTRFS_I(inode),
+					   ilock_flags | BTRFS_ILOCK_SHARED);
+			has_shared_lock = false;
+			goto relock;
+		}
+
 		btrfs_inode_unlock(BTRFS_I(inode), ilock_flags);
 		goto out;
 	}
@@ -1496,6 +1516,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;
+		has_shared_lock = false;
 		goto relock;
 	}
 
@@ -1632,7 +1653,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;
 
-- 
2.39.2


  parent reply	other threads:[~2023-08-30 22:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-30 18:15 [PATCH 0/2] Use exclusive lock for file_remove_privs Bernd Schubert
2023-08-30 18:15 ` [PATCH 1/2] fs: Add and export file_needs_remove_privs Bernd Schubert
2023-09-01  6:48   ` Christoph Hellwig
2023-08-30 18:15 ` Bernd Schubert [this message]
2023-08-31  7:40 ` [PATCH 0/2] Use exclusive lock for file_remove_privs Christoph Hellwig
2023-08-31 10:17   ` Bernd Schubert
2023-08-31 10:18 ` Mateusz Guzik
2023-08-31 14:41   ` Bernd Schubert
2023-09-01  6:49   ` Christoph Hellwig
2023-09-01 11:38   ` Matthew Wilcox
2023-09-01 11:47     ` Mateusz Guzik
2023-09-06 15:13       ` Matthew Wilcox
  -- strict thread matches above, loose matches on Subject: below --
2023-08-31 11:24 [PATCH v2 " Bernd Schubert
2023-08-31 11:24 ` [PATCH 2/2] btrfs: file_remove_privs needs an exclusive lock Bernd Schubert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230830181519.2964941-3-bschubert@ddn.com \
    --to=bschubert@ddn.com \
    --cc=bernd.schubert@fastmail.fm \
    --cc=clm@fb.com \
    --cc=dsingh@ddn.com \
    --cc=dsterba@suse.com \
    --cc=josef@toxicpanda.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=rgoldwyn@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox