| Message ID | 20211123151812.361624-1-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs/iomap: Fix write path page prefaulting | expand |
On Tue, Nov 23, 2021 at 04:18:12PM +0100, Andreas Gruenbacher wrote: > When part of the user buffer passed to generic_perform_write() or > iomap_file_buffered_write() cannot be faulted in for reading, the entire > write currently fails. > > The correct behavior would be to write all the data that can be written, > up to the point of failure. Since commit a6294593e8a1 ("iov_iter: Turn > iov_iter_fault_in_readable into fault_in_iov_iter_readable"), we have > enough information to implement that, so change the code to do that. > > We already take into account that pages faulted in may no longer be > resident by the time they are accessed, so the code will also behave > correctly when part of the buffer isn't faulted in in the first place. > > This leads to an intentional user-visible change when the buffer passed > to write calls contains unmapped or poisoned pages. > > (This change obsoletes commit 554c577cee95 ("gfs2: Prevent endless loops > in gfs2_file_buffered_write").) > > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> FWIW: Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> This would be more consistent as in my tests on a vanilla kernel ext4 and btrfs fail with EFAULT while gfs2 copies as much as it can before hitting the fault.
Looks fine,
Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 1753c26c8e76..54516ab464cd 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -750,7 +750,7 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i) * same page as we're writing to, without it being marked * up-to-date. */ - if (unlikely(fault_in_iov_iter_readable(i, bytes))) { + if (unlikely(fault_in_iov_iter_readable(i, bytes) == bytes)) { status = -EFAULT; break; } diff --git a/mm/filemap.c b/mm/filemap.c index daa0e23a6ee6..767202265048 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -3743,7 +3743,7 @@ ssize_t generic_perform_write(struct file *file, * same page as we're writing to, without it being marked * up-to-date. */ - if (unlikely(fault_in_iov_iter_readable(i, bytes))) { + if (unlikely(fault_in_iov_iter_readable(i, bytes) == bytes)) { status = -EFAULT; break; }
When part of the user buffer passed to generic_perform_write() or iomap_file_buffered_write() cannot be faulted in for reading, the entire write currently fails. The correct behavior would be to write all the data that can be written, up to the point of failure. Since commit a6294593e8a1 ("iov_iter: Turn iov_iter_fault_in_readable into fault_in_iov_iter_readable"), we have enough information to implement that, so change the code to do that. We already take into account that pages faulted in may no longer be resident by the time they are accessed, so the code will also behave correctly when part of the buffer isn't faulted in in the first place. This leads to an intentional user-visible change when the buffer passed to write calls contains unmapped or poisoned pages. (This change obsoletes commit 554c577cee95 ("gfs2: Prevent endless loops in gfs2_file_buffered_write").) Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- fs/iomap/buffered-io.c | 2 +- mm/filemap.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)