[PATCH 5/7] bcachefs: Move prefaulting out of hot write path

[Dave Hansen]

From: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxx>
Cc: Kent Overstreet <kent.overstreet@xxxxxxxxx>
Cc: linux-bcachefs@xxxxxxxxxxxxxxx

---

 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;
_