Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.16] 'struct iov_iter' has no member named 'type' #12757

Closed
65a opened this issue Nov 12, 2021 · 15 comments
Closed

[5.16] 'struct iov_iter' has no member named 'type' #12757

65a opened this issue Nov 12, 2021 · 15 comments
Labels
Type: Building Indicates an issue related to building binaries Type: Defect Incorrect behavior (e.g. crash, hang)

Comments

@65a
Copy link

65a commented Nov 12, 2021

zpl_file.c:255:15: error: 'struct iov_iter' has no member named 'type'

@65a 65a added the Type: Defect Incorrect behavior (e.g. crash, hang) label Nov 12, 2021
@65a
Copy link
Author

65a commented Nov 12, 2021

torvalds/linux@8cd54c1

@joshniec
Copy link

What version of zfs and os/kernel are you using?

Similar PR implements a fix for the exact error you are encountering: #11411

@65a
Copy link
Author

65a commented Nov 14, 2021

See the commit in my second comment. The struct has changed in torvalds/linux HEAD. This change postdates the PR you are referencing. Thanks!

@jgottula
Copy link
Contributor

In the future, it'd be very helpful if you used the standard issue template, providing the Linux kernel version you're using as well as the ZFS version you're using. Or answered the reply asking which versions you're using. (That would have made it much easier to determine that you were actually building for a Linux kernel from the master branch, containing merge-window changes for v5.16-rc1 in it, rather than a 5.15 kernel.)

The title of this issue should be changed to reflect that this is actually a 5.16 compat breakage.


Anyway... after figuring out that this wasn't actually a 5.15 kernel build, I manually traced out what's going on: it looks like the configure script incorrectly determined that HAVE_VFS_IOV_ITER is false for your kernel; and as a consequence, the #else branch of the #if defined(HAVE_VFS_IOV_ITER) conditional in zpl_uio_init is taken (something which was intended for older kernels).

The mentioned commit torvalds/linux@8cd54c1 is not actually terribly relevant; that change has been there since kernel 5.14.

The relevant commit here appears to be torvalds/linux@a629459. (Which, as mentioned previously, is a Linux 5.16 commit.)

  • The commit replaces iov_iter_fault_in_readable with fault_in_iov_iter_readable.
  • This causes the ZFS check for HAVE_IOV_ITER_FAULT_IN_READABLE to fail.
  • The ZFS check for HAVE_VFS_IOV_ITER also fails, as it is a conjunction of multiple conditionals, including the one for HAVE_IOV_ITER_FAULT_IN_READABLE.

It would probably be helpful if you could attach the terminal output from the ZFS configure script, or better yet, the config.log file. There's a low likelihood that perhaps more than one of the seven required checks for HAVE_VFS_IOV_ITER might have broken, and that output would determine whether that's the case.

@65a 65a changed the title [5.15] 'struct iov_iter' has no member named 'type' [5.16] 'struct iov_iter' has no member named 'type' Nov 16, 2021
@65a
Copy link
Author

65a commented Nov 16, 2021

Titles updated, point taken. I am not using a distribution, and I generally am building the kernel from HEAD and ZFS from HEAD in a greenfield build. I'll use template next time. Please consider using CI, I'd prefer to be replaced by a bot :)

@jgottula
Copy link
Contributor

Please consider using CI, I'd prefer to be replaced by a bot :)

Yeah I don't actually know what the "official" new-kernel-version compat procedure is around here, or if there necessarily is a set procedure. (I'm just a lowly contributor-of-a-few-patches-and-some-issue-reports-and-stuff.)

Btw, sorry if the criticism came across as harsh. It probably would have helped if I had gone thru the issues in ascending order, rather than descending order; since HEAD was mentioned in at least one of the lower-numbered issues. In any case, it's helpful to know about upcoming kernel incompatibilities. 👍

@65a
Copy link
Author

65a commented Nov 17, 2021

The error is mine for saying 5.15 when the 5.16 merge window was happening, and filing bugs on Friday evening when my local builds weren't working :) Generally, I'm surprised when I end up with build errors, which is saying a lot. Glad to help concoct a build script to act as early warning, which was my goal here...if there's a better channel than github issues, glad to participate there.

@satmandu
Copy link
Contributor

satmandu commented Nov 29, 2021

I took an initial crack at this but got this new error:

/build/module/zfs/../os/linux/zfs/zvol_os.c:815:35: error: initialization of ‘void (*)(struct bio *)’ from incompatible pointer type ‘blk_qc_t (*)(struct bio *)’ {aka ‘unsigned int (*)(struct bio *)’} [-Werror=incompatible-pointer-types]
  815 |         .submit_bio             = zvol_submit_bio,
      |                                   ^~~~~~~~~~~~~~~

Maybe one of you can tell me what needs fixing?

--- a/config/kernel-vfs-iov_iter.m4
+++ b/config/kernel-vfs-iov_iter.m4
@@ -41,6 +41,17 @@
 		error = iov_iter_fault_in_readable(&iter, size);
 	])
 
+	ZFS_LINUX_TEST_SRC([fault_in_iov_iter_readable], [
+		#include <linux/fs.h>
+		#include <linux/uio.h>
+	],[
+		struct iov_iter iter = { 0 };
+		size_t size = 512;
+		int error __attribute__ ((unused));
+
+		error = fault_in_iov_iter_readable(&iter, size);
+	])
+
 	ZFS_LINUX_TEST_SRC([iov_iter_count], [
 		#include <linux/fs.h>
 		#include <linux/uio.h>
@@ -78,6 +89,7 @@
 
 AC_DEFUN([ZFS_AC_KERNEL_VFS_IOV_ITER], [
 	enable_vfs_iov_iter="yes"
+	fault_iov_iter="no"
 
 	AC_MSG_CHECKING([whether iov_iter types are available])
 	ZFS_LINUX_TEST_RESULT([iov_iter_types], [
@@ -114,11 +126,23 @@
 		AC_MSG_RESULT(yes)
 		AC_DEFINE(HAVE_IOV_ITER_FAULT_IN_READABLE, 1,
 		    [iov_iter_fault_in_readable() is available])
+		fault_iov_iter="yes"
 	],[
 		AC_MSG_RESULT(no)
-		enable_vfs_iov_iter="no"
 	])
 
+	AC_MSG_CHECKING([whether fault_in_iov_iter_readable() is available])
+	ZFS_LINUX_TEST_RESULT([fault_in_iov_iter_readable], [
+		AC_MSG_RESULT(yes)
+		AC_DEFINE(HAVE_FAULT_IN_IOV_ITER_READABLE, 1,
+		   [fault_in_iov_iter_readable() is available])
+		fault_iov_iter="yes"
+	],[
+		AC_MSG_RESULT(no)
+	])
+
+	AS_IF([test "x$fault_iov_iter" = xno],[enable_vfs_iov_iter="no"])
+
 	AC_MSG_CHECKING([whether iov_iter_count() is available])
 	ZFS_LINUX_TEST_RESULT([iov_iter_count], [
 		AC_MSG_RESULT(yes)
--- a/module/os/linux/zfs/zfs_uio.c
+++ b/module/os/linux/zfs/zfs_uio.c
@@ -221,7 +221,7 @@
 	if (uio->uio_segflg == UIO_SYSSPACE || uio->uio_segflg == UIO_BVEC) {
 		/* There's never a need to fault in kernel pages */
 		return (0);
-#if defined(HAVE_VFS_IOV_ITER)
+#if defined(HAVE_VFS_IOV_ITER) && defined(HAVE_IOV_ITER_FAULT_IN_READABLE)
 	} else if (uio->uio_segflg == UIO_ITER) {
 		/*
 		 * At least a Linux 4.9 kernel, iov_iter_fault_in_readable()
@@ -229,6 +229,13 @@
 		 */
 		if (iov_iter_fault_in_readable(uio->uio_iter, n))
 			return (EFAULT);
+#elif defined(HAVE_VFS_IOV_ITER) && defined(HAVE_FAULT_IN_IOV_ITER_READABLE)
+	} else if (uio->uio_segflg == UIO_ITER) {
+		/*
+		* At least a Linux 5.16 kernel, fault_in_iov_iter_readable()
+		*/
+		if (fault_in_iov_iter_readable(uio->uio_iter, n))
+			return (EFAULT);
 #endif
 	} else {
 		/* Fault in all user pages */
--- a/zfs_config.h.in
+++ b/zfs_config.h.in
@@ -341,6 +341,9 @@
 /* iov_iter_fault_in_readable() is available */
 #undef HAVE_IOV_ITER_FAULT_IN_READABLE
 
+/* fault_in_iov_iter_readable() is available */
+#undef HAVE_FAULT_IN_IOV_ITER_READABLE
+
 /* iov_iter_revert() is available */
 #undef HAVE_IOV_ITER_REVERT

@behlendorf
Copy link
Contributor

cc: @ckane

@behlendorf behlendorf added the Type: Building Indicates an issue related to building binaries label Nov 30, 2021
@jgottula
Copy link
Contributor

jgottula commented Dec 2, 2021

I took an initial crack at this but got this new error:

/build/module/zfs/../os/linux/zfs/zvol_os.c:815:35: error: initialization of ‘void (*)(struct bio *)’ from incompatible pointer type ‘blk_qc_t (*)(struct bio *)’ {aka ‘unsigned int (*)(struct bio *)’} [-Werror=incompatible-pointer-types]
  815 |         .submit_bio             = zvol_submit_bio,
      |                                   ^~~~~~~~~~~~~~~

Maybe one of you can tell me what needs fixing?

Oh. This seems to be another consequence of the change to the submit_bio prototype that happened in torvalds/linux@3e08773.

We already have a merged commit (453c63e) fixing a separate instance of something being broken by that particular upstream commit. In that case, it was possible to be clever and simply use a cast-to-void to handle both return type possibilities.

But now that we know the definition of function zvol_submit_bio is also affected—and so therefore we will need a proper explicit #ifdef HAVE_VOID_RETURNING_SUBMIT_BIO (or whatever) conditional for the function return type—perhaps 453c63e warrants a revisit. (Definitely the config script side of the commit, anyway. The cast-to-void in vdev_submit_bio_impl itself seems arguably fine as-is; I suppose it just comes down to how explicit we want to be, now that we know there will be an actual config conditional for the submit_bio return type.)

@ckane
Copy link
Contributor

ckane commented Dec 2, 2021

I do have this fix (but not yet hooked up in autotools) in my tree. Basically need to conditionally compile the following when the submit_bio return type is void:

diff --git a/module/os/linux/zfs/zvol_os.c b/module/os/linux/zfs/zvol_os.c
index c17423426..dfb3644ba 100644
--- a/module/os/linux/zfs/zvol_os.c
+++ b/module/os/linux/zfs/zvol_os.c
@@ -762,7 +762,7 @@ static struct block_device_operations zvol_ops = {
        .getgeo                 = zvol_getgeo,
        .owner                  = THIS_MODULE,
 #ifdef HAVE_SUBMIT_BIO_IN_BLOCK_DEVICE_OPERATIONS
-       .submit_bio             = zvol_submit_bio,
+       .submit_bio             = (void(*)(struct bio *))zvol_submit_bio,
 #endif
 };

@ckane
Copy link
Contributor

ckane commented Dec 3, 2021

This change appears to fix the submit_bio type mismatch:
ckane@bd8b115

@ckane
Copy link
Contributor

ckane commented Dec 3, 2021

Likely fixed in #12819

@szubersk
Copy link
Contributor

@65a Seems like current master builds with kernel 5.16 perfectly fine. Do you confirm?

@behlendorf
Copy link
Contributor

I'm going to go ahead and close this. This build issue was resolved in master and the 2.1.3 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Building Indicates an issue related to building binaries Type: Defect Incorrect behavior (e.g. crash, hang)
Projects
None yet
Development

No branches or pull requests

7 participants