public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@linux.intel.com>
To: linux-kernel@vger.kernel.org
Cc: Linus Torvalds <torvalds@linux-foundation.org>,Ted Ts'o
	<tytso@mit.edu>,Christian Brauner <brauner@kernel.org>,Darrick
	J. Wong <djwong@kernel.org>,Matthew Wilcox (Oracle)
	<willy@infradead.org>,Al Viro
	<viro@zeniv.linux.org.uk>,linux-fsdevel@vger.kernel.org,Dave
	Hansen
	<dave.hansen@linux.intel.com>,kent.overstreet@linux.dev,linux-bcachefs@vger.kernel.org
Subject: [PATCH 5/7] bcachefs: Move prefaulting out of hot write path
Date: Wed, 29 Jan 2025 10:17:58 -0800	[thread overview]
Message-ID: <20250129181758.ACB89DF8@davehans-spike.ostc.intel.com> (raw)
In-Reply-To: <20250129181749.C229F6F3@davehans-spike.ostc.intel.com>


From: Dave Hansen <dave.hansen@linux.intel.com>

Prefaulting the write source buffer incurs an extra userspace access
in the common fast path. Make bch2_buffered_write() consistent with
generic_perform_write(): only touch userspace an extra time when
copy_page_from_iter_atomic() has failed to make progress.

This also zaps a comment. It referred to a possible deadlock and to
userspace address checks. Neither of those things are a concern when
using copy_folio_from_iter_atomic() for atomic usercopies. It
prevents deadlocks by disabling page faults and it leverages user
copy functions that have their own access_ok() checks.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Kent Overstreet <kent.overstreet@linux.dev>
Cc: linux-bcachefs@vger.kernel.org

---

 b/fs/bcachefs/fs-io-buffered.c |   30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff -puN fs/bcachefs/fs-io-buffered.c~bcachefs-postfault fs/bcachefs/fs-io-buffered.c
--- a/fs/bcachefs/fs-io-buffered.c~bcachefs-postfault	2025-01-29 09:03:35.727656612 -0800
+++ b/fs/bcachefs/fs-io-buffered.c	2025-01-29 09:03:35.731656945 -0800
@@ -970,26 +970,6 @@ static ssize_t bch2_buffered_write(struc
 		unsigned offset = pos & (PAGE_SIZE - 1);
 		unsigned bytes = iov_iter_count(iter);
 again:
-		/*
-		 * Bring in the user page that we will copy from _first_.
-		 * Otherwise there's a nasty deadlock on copying from the
-		 * same page as we're writing to, without it being marked
-		 * up-to-date.
-		 *
-		 * Not only is this an optimisation, but it is also required
-		 * to check that the address is actually valid, when atomic
-		 * usercopies are used, below.
-		 */
-		if (unlikely(fault_in_iov_iter_readable(iter, bytes))) {
-			bytes = min_t(unsigned long, iov_iter_count(iter),
-				      PAGE_SIZE - offset);
-
-			if (unlikely(fault_in_iov_iter_readable(iter, bytes))) {
-				ret = -EFAULT;
-				break;
-			}
-		}
-
 		if (unlikely(fatal_signal_pending(current))) {
 			ret = -EINTR;
 			break;
@@ -1012,6 +992,16 @@ again:
 			 */
 			bytes = min_t(unsigned long, PAGE_SIZE - offset,
 				      iov_iter_single_seg_count(iter));
+
+			/*
+			 * Faulting in 'iter' may be required for forward
+			 * progress. Do it here, out outside the fast path
+			 * and when not holding any folio locks.
+			 */
+			if (fault_in_iov_iter_readable(iter, bytes) == bytes) {
+				ret = -EFAULT;
+				break;
+			}
 			goto again;
 		}
 		pos += ret;
_

  parent reply	other threads:[~2025-01-29 18:17 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-29 18:17 [PATCH 0/7] Move prefaulting into write slow paths Dave Hansen
2025-01-29 18:17 ` [PATCH 1/7] filemap: Move prefaulting out of hot write path Dave Hansen
2025-01-29 18:17 ` [PATCH 2/7] iomap: " Dave Hansen
2025-01-31  7:59   ` Christoph Hellwig
2025-01-29 18:17 ` [PATCH 3/7] ntfs3: " Dave Hansen
2025-01-29 18:17 ` [PATCH 4/7] fuse: " Dave Hansen
2025-04-15  8:43   ` Miklos Szeredi
2025-01-29 18:17 ` Dave Hansen [this message]
2025-01-29 18:18 ` [PATCH 6/7] btrfs: " Dave Hansen
2025-01-29 18:18 ` [PATCH 7/7] netfs: Remove outdated comments about prefaulting Dave Hansen
2025-01-30  7:44 ` [PATCH 0/7] Move prefaulting into write slow paths Kent Overstreet
2025-01-30 16:04   ` Dave Hansen
2025-01-30 21:36     ` Dave Chinner
2025-01-31  1:06       ` Kent Overstreet
2025-01-31  0:56     ` Kent Overstreet
2025-01-31  1:34       ` Dave Hansen
2025-01-31  2:17         ` Kent Overstreet

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=20250129181758.ACB89DF8@davehans-spike.ostc.intel.com \
    --to=dave.hansen@linux.intel.com \
    --cc=brauner@kernel.org \
    --cc=djwong@kernel.org \
    --cc=kent.overstreet@linux.dev \
    --cc=linux-bcachefs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /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