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