iov_iter_fault_in_readable is now fault_in_iov_iter_readable

[sempervictus]

Just caught a crash on my in-tree build:

  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND objtool
  CHK     include/generated/compile.h
  CC      fs/zfs/zfs/../os/linux/zfs/zfs_uio.o
fs/zfs/zfs/../os/linux/zfs/zfs_uio.c: In function ‘zfs_uio_prefaultpages’:
fs/zfs/zfs/../os/linux/zfs/zfs_uio.c:364:21: error: implicit declaration of function ‘iov_iter_fault_in_readable’ [-Werror=implicit-function-declaration]
  364 |                 if (iov_iter_fault_in_readable(uio->uio_iter, n))
      |                     ^~~~~~~~~~~~~~~~~~~~~~~~~~

someone smarter than me pointed out that this recently hit mainline:

commit 30e66b1dfcbbe409c76500a77ecd20b3cf5b8fa5
Author: Andreas Gruenbacher <[email protected]>
Date:   Fri Apr 15 06:28:40 2022 +0800

    iov_iter: Turn iov_iter_fault_in_readable into fault_in_iov_iter_readable
    
    commit a6294593e8a1290091d0b078d5d33da5e0cd3dfe upstream
    
    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.
    
    Rename iov_iter_fault_in_readable to fault_in_iov_iter_readable to make
    sure this change doesn't silently break things.
    
    Signed-off-by: Andreas Gruenbacher <[email protected]>
    Signed-off-by: Anand Jain <[email protected]>
    Signed-off-by: Greg Kroah-Hartman <[email protected]>

So we need to have that properly handled in the autoconf pieces and source.
I'm going to try doing a gross patch of the in-kernel code to replace the function with the new one in the hopes that it works correctly (lets hope ztest catches this), but i won't have time to do the detailed build system work for a bit and this is going to bite others shortly.

[sempervictus]

My initial (untested) hack-around the problem:

commit dadbf8fdafae4cc880baf5562d30f41cf4ac0625 (HEAD -> sv5.15.37-grsec)
Author: RageLtMan <rageltman [at] sempervictus>
Date:   Sun May 1 21:35:55 2022 -0400

    ZFS: Replace iov_iter_fault_in_readable
    
    30e66b1dfcbb removed iov_iter_fault_in_readable and replaced it
    with fault_in_iov_iter_readable.
    
    Try to hot-swap the functions in the hope that their returns are
    compatible-enough with the design of this code-path to permit
    direct substitution without changing the logic around this path.

diff --git a/fs/zfs/os/linux/zfs/zfs_uio.c b/fs/zfs/os/linux/zfs/zfs_uio.c
index 3ac8645fe4db..c276176347df 100644
--- a/fs/zfs/os/linux/zfs/zfs_uio.c
+++ b/fs/zfs/os/linux/zfs/zfs_uio.c
@@ -358,10 +358,10 @@ zfs_uio_prefaultpages(ssize_t n, zfs_uio_t *uio)
 #if defined(HAVE_VFS_IOV_ITER)
        } else if (uio->uio_segflg == UIO_ITER) {
                /*
-                * At least a Linux 4.9 kernel, iov_iter_fault_in_readable()
+                * At least a Linux 5.15.37 kernel, fault_in_iov_iter_readable()
                 * can be relied on to fault in user pages when referenced.
                 */
-               if (iov_iter_fault_in_readable(uio->uio_iter, n))
+               if (fault_in_iov_iter_readable(uio->uio_iter, n))
                        return (EFAULT);
 #endif

[rincebrain]

#12975 should have a rewrite header for that; perhaps you could revise that to figure out why it's not working now?

[sempervictus]

@rincebrain - a bit "all out" on other efforts right now, but i'm guessing the AC code does a version check not a functional check, because i just re-ran the copy in-tree routine and diffing out from my fixed branch i see:

diff --git c/fs/zfs/os/linux/zfs/zfs_uio.c w/fs/zfs/os/linux/zfs/zfs_uio.c
index c276176347df..3ac8645fe4db 100644
--- c/fs/zfs/os/linux/zfs/zfs_uio.c
+++ w/fs/zfs/os/linux/zfs/zfs_uio.c
@@ -358,10 +358,10 @@ zfs_uio_prefaultpages(ssize_t n, zfs_uio_t *uio)
 #if defined(HAVE_VFS_IOV_ITER)
        } else if (uio->uio_segflg == UIO_ITER) {
                /*
-                * At least a Linux 5.15.37 kernel, fault_in_iov_iter_readable()
+                * At least a Linux 4.9 kernel, iov_iter_fault_in_readable()
                 * can be relied on to fault in user pages when referenced.
                 */
-               if (fault_in_iov_iter_readable(uio->uio_iter, n))
+               if (iov_iter_fault_in_readable(uio->uio_iter, n))
                        return (EFAULT);
 #endif
        } else {

[rincebrain]

It just generates a #define [old] [new] in a header if it detects the latter existing, so it not changing the callsite isn't surprising.

[behlendorf]

If you can manually run the configure check that will show why it didn't detect the new interface. After running configure once you can just cut-and-paste the make line at the top of the build/fault_in_iov_iter_readable/Makefile. From what I can tell it does appear to correctly detect the interface with a vanilla kernel.

[stale]

This issue has been automatically marked as "stale" because it has not had any activity for a while. It will be closed in 90 days if no further activity occurs. Thank you for your contributions.

[behlendorf]

@sempervictus is this still an issue with certain kernels?