On Fri, Aug 27, 2021 at 11:56:44PM +0300, Kari Argillander wrote:
> On Fri, Aug 27, 2021 at 06:49:11PM +0200, Andreas Gruenbacher wrote:
> > Turn iov_iter_fault_in_readable into a function that returns the number
> > of bytes not faulted in (similar to copy_to_user) instead of returning a
> > non-zero value when any of the requested pages couldn't be faulted in.
> > This supports the existing users that require all pages to be faulted in
> > as well as new users that are happy if any pages can be faulted in at
> > all.
> >
> > Rename iov_iter_fault_in_readable to fault_in_iov_iter_readable to make
> > sure that this change doesn't silently break things.
>
> At least this patch will break ntfs3 which is in next. It has been there
> just couple weeks so I understand. I added Konstantin and ntfs3 list so
> that we know what is going on. Can you please info if and when do we
> need rebase.
>
> We are in situation that ntfs3 might get in 5.15, but it is uncertain so
> it would be best that we solve this. Just info is enough.
I forget to add permalink for the thread.
lore.kernel.org/linux-fsdevel/20210827164926.1726765-1-agruenba@xxxxxxxxxx/
Please do not respond this message. I dropped people from CC.
>
> Argillander
>
> >
> > Signed-off-by: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
> > ---
> > fs/btrfs/file.c | 2 +-
> > fs/f2fs/file.c | 2 +-
> > fs/fuse/file.c | 2 +-
> > fs/iomap/buffered-io.c | 2 +-
> > fs/ntfs/file.c | 2 +-
> > include/linux/uio.h | 2 +-
> > lib/iov_iter.c | 33 +++++++++++++++++++++------------
> > mm/filemap.c | 2 +-
> > 8 files changed, 28 insertions(+), 19 deletions(-)
> >
> > diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> > index ee34497500e1..281c77cfe91a 100644
> > --- a/fs/btrfs/file.c
> > +++ b/fs/btrfs/file.c
> > @@ -1698,7 +1698,7 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
> > * Fault pages before locking them in prepare_pages
> > * to avoid recursive lock
> > */
> > - if (unlikely(iov_iter_fault_in_readable(i, write_bytes))) {
> > + if (unlikely(fault_in_iov_iter_readable(i, write_bytes))) {
> > ret = -EFAULT;
> > break;
> > }
> > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> > index 6afd4562335f..b04b6c909a8b 100644
> > --- a/fs/f2fs/file.c
> > +++ b/fs/f2fs/file.c
> > @@ -4259,7 +4259,7 @@ static ssize_t f2fs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> > size_t target_size = 0;
> > int err;
> >
> > - if (iov_iter_fault_in_readable(from, iov_iter_count(from)))
> > + if (fault_in_iov_iter_readable(from, iov_iter_count(from)))
> > set_inode_flag(inode, FI_NO_PREALLOC);
> >
> > if ((iocb->ki_flags & IOCB_NOWAIT)) {
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 97f860cfc195..da49ef71dab5 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -1160,7 +1160,7 @@ static ssize_t fuse_fill_write_pages(struct fuse_io_args *ia,
> >
> > again:
> > err = -EFAULT;
> > - if (iov_iter_fault_in_readable(ii, bytes))
> > + if (fault_in_iov_iter_readable(ii, bytes))
> > break;
> >
> > err = -ENOMEM;
> > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > index 87ccb3438bec..7dc42dd3a724 100644
> > --- a/fs/iomap/buffered-io.c
> > +++ b/fs/iomap/buffered-io.c
> > @@ -749,7 +749,7 @@ iomap_write_actor(struct inode *inode, loff_t pos, loff_t length, void *data,
> > * same page as we're writing to, without it being marked
> > * up-to-date.
> > */
> > - if (unlikely(iov_iter_fault_in_readable(i, bytes))) {
> > + if (unlikely(fault_in_iov_iter_readable(i, bytes))) {
> > status = -EFAULT;
> > break;
> > }
> > diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> > index ab4f3362466d..a43adeacd930 100644
> > --- a/fs/ntfs/file.c
> > +++ b/fs/ntfs/file.c
> > @@ -1829,7 +1829,7 @@ static ssize_t ntfs_perform_write(struct file *file, struct iov_iter *i,
> > * pages being swapped out between us bringing them into memory
> > * and doing the actual copying.
> > */
> > - if (unlikely(iov_iter_fault_in_readable(i, bytes))) {
> > + if (unlikely(fault_in_iov_iter_readable(i, bytes))) {
> > status = -EFAULT;
> > break;
> > }
> > diff --git a/include/linux/uio.h b/include/linux/uio.h
> > index 82c3c3e819e0..12d30246c2e9 100644
> > --- a/include/linux/uio.h
> > +++ b/include/linux/uio.h
> > @@ -119,7 +119,7 @@ size_t copy_page_from_iter_atomic(struct page *page, unsigned offset,
> > size_t bytes, struct iov_iter *i);
> > void iov_iter_advance(struct iov_iter *i, size_t bytes);
> > void iov_iter_revert(struct iov_iter *i, size_t bytes);
> > -int iov_iter_fault_in_readable(const struct iov_iter *i, size_t bytes);
> > +size_t fault_in_iov_iter_readable(const struct iov_iter *i, size_t bytes);
> > size_t iov_iter_single_seg_count(const struct iov_iter *i);
> > size_t copy_page_to_iter(struct page *page, size_t offset, size_t bytes,
> > struct iov_iter *i);
> > diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> > index 069cedd9d7b4..082ab155496d 100644
> > --- a/lib/iov_iter.c
> > +++ b/lib/iov_iter.c
> > @@ -430,33 +430,42 @@ static size_t copy_page_to_iter_pipe(struct page *page, size_t offset, size_t by
> > }
> >
> > /*
> > + * fault_in_iov_iter_readable - fault in iov iterator for reading
> > + * @i: iterator
> > + * @size: maximum length
> > + *
> > * Fault in one or more iovecs of the given iov_iter, to a maximum length of
> > - * bytes. For each iovec, fault in each page that constitutes the iovec.
> > + * @size. For each iovec, fault in each page that constitutes the iovec.
> > + *
> > + * Returns the number of bytes not faulted in (like copy_to_user() and
> > + * copy_from_user()).
> > *
> > - * Return 0 on success, or non-zero if the memory could not be accessed (i.e.
> > - * because it is an invalid address).
> > + * Always returns 0 for non-userspace iterators.
> > */
> > -int iov_iter_fault_in_readable(const struct iov_iter *i, size_t bytes)
> > +size_t fault_in_iov_iter_readable(const struct iov_iter *i, size_t size)
> > {
> > if (iter_is_iovec(i)) {
> > + size_t count = min(size, iov_iter_count(i));
> > const struct iovec *p;
> > size_t skip;
> >
> > - if (bytes > i->count)
> > - bytes = i->count;
> > - for (p = i->iov, skip = i->iov_offset; bytes; p++, skip = 0) {
> > - size_t len = min(bytes, p->iov_len - skip);
> > + size -= count;
> > + for (p = i->iov, skip = i->iov_offset; count; p++, skip = 0) {
> > + size_t len = min(count, p->iov_len - skip);
> > + size_t ret;
> >
> > if (unlikely(!len))
> > continue;
> > - if (fault_in_readable(p->iov_base + skip, len))
> > - return -EFAULT;
> > - bytes -= len;
> > + ret = fault_in_readable(p->iov_base + skip, len);
> > + count -= len - ret;
> > + if (ret)
> > + break;
> > }
> > + return count + size;
> > }
> > return 0;
> > }
> > -EXPORT_SYMBOL(iov_iter_fault_in_readable);
> > +EXPORT_SYMBOL(fault_in_iov_iter_readable);
> >
> > void iov_iter_init(struct iov_iter *i, unsigned int direction,
> > const struct iovec *iov, unsigned long nr_segs,
> > diff --git a/mm/filemap.c b/mm/filemap.c
> > index 4dec3bc7752e..83af8a534339 100644
> > --- a/mm/filemap.c
> > +++ b/mm/filemap.c
> > @@ -3643,7 +3643,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(iov_iter_fault_in_readable(i, bytes))) {
> > + if (unlikely(fault_in_iov_iter_readable(i, bytes))) {
> > status = -EFAULT;
> > break;
> > }
> > --
> > 2.26.3
> >
>
