From: Dave Hansen <[email protected]>
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 <[email protected]> Cc: Kent Overstreet <[email protected]> Cc: [email protected] --- 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; _
