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