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

Linux 5.11 compat #11390

Closed
wants to merge 6 commits into from
Closed

Conversation

behlendorf
Copy link
Contributor

@behlendorf behlendorf commented Dec 23, 2020

Motivation and Context

Issue #11387. Linux 5.11 compat.

Description

The Linux 5.11 kernel has made multiple changes to the block layer
interfaces which require some new compatibility code. This PR contains
a stack of 5 patches each of which addresses a different block device
interface change. An explanation of each commit is in the associated
commit comment.

  • Linux 5.11 compat: lookup_bdev()
  • Linux 5.11 compat: bio_start_io_acct() / bio_end_io_acct()
  • Linux 5.11 compat: bdev_whole()
  • Linux 5.11 compat: revalidate_disk_size()
  • Linux 5.11 compat: blk_{un}register_region()

How Has This Been Tested?

Compiled on a 3.10 (CentOS 7) and unreleased 5.11 mainline kernel.
Ran the sanity.run ZTS tests to verify basic functionality. The CI will
provide test coverage for a wide range of kernels and I'm planning to
run the full ZTS suite locally with the 5.11 kernel.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf behlendorf added Type: Building Indicates an issue related to building binaries Status: Code Review Needed Ready for review and testing labels Dec 23, 2020
@ckane
Copy link
Contributor

ckane commented Dec 23, 2020

Hey @behlendorf - looks like on Dec 1 they made MODULE_LICENSE an error instead of a warn, which makes the kernel module build tests fail in config/kernel.m4 because the conftest.c doesn't get those things. I modified the ZFS_LINUX_TEST_PROGRAM per below and I think that should get it. Going to test with both change sets merged later today:

dnl #
dnl # ZFS_LINUX_TEST_PROGRAM(C)([PROLOGUE], [BODY])
dnl #
m4_define([ZFS_LINUX_TEST_PROGRAM], [
$1
#include <linux/module.h>
int
main (void)
{
$2
        ;
        return 0;
};
MODULE_DESCRIPTION("conftest");
MODULE_AUTHOR(ZFS_META_AUTHOR);
MODULE_LICENSE(ZFS_META_LICENSE);
MODULE_VERSION(ZFS_META_VERSION "-" ZFS_META_RELEASE);
])

@ckane
Copy link
Contributor

ckane commented Dec 23, 2020

Also, I get the following error when running configure against the latest Linux git source tree:

checking whether PDE_DATA() is available... configure: error:
	*** None of the expected "PDE_DATA" interfaces were detected.
	*** This may be because your kernel version is newer than what is
	*** supported, or you are using a patched custom kernel with
	*** incompatible modifications.
	***
	*** ZFS Version: zfs-2.0.0-rc1
	*** Compatible Kernels: 3.10 - 5.9

However, when I go into build/pde_data/ and run the following it works fine, and I can't gather any meaningful output from the log files:

make modules -C /<.....>/git/kernels/linux  M=/<.....>/src/zfs-test/build/pde_data

This is against linux.git commit 614cb5894306cfa2c7d9b6168182876ff5948735.

@behlendorf
Copy link
Contributor Author

@ckane could you point me at the relevant kernel commits. I wasn't able to find them.

@ckane
Copy link
Contributor

ckane commented Dec 23, 2020

@ckane could you point me at the relevant kernel commits. I wasn't able to find them.

Sure, here are (what I think) are relevant changes:

  • torvalds/linux@1d6cd39 (require MODULE_LICENSE)
  • torvalds/linux@d6d692f (GPL-incompatibility becomes fatal - not sure if this might be contributing to the PDE_DATA error, but I do see some complaints about this too in logs)

@behlendorf
Copy link
Contributor Author

@ckane thanks for the links. It looks like those changes were only merged yesterday which is why I hadn't seen them yet. I've added another commit to the stack to handle the change from a warning to error. Would you mind testing the updated stack in your environment.

@ckane
Copy link
Contributor

ckane commented Dec 23, 2020

Thanks, this appears to build on both the new kernel and older kernels for me.

config/kernel-blkdev.m4 Outdated Show resolved Hide resolved
Update the ZFS_LINUX_TEST_PROGRAM macro to always set the module
license.  As of the 5.11 kernel not setting a license has been
converted from a warning to an error.

Signed-off-by: Brian Behlendorf <[email protected]>
The lookup_bdev() function has been updated to require a dev_t
be passed as the second argument. This is actually pretty nice
since the major number stored in the dev_t was the only part we
were interested in. This allows to us avoid handling the bdev
entirely.  The vdev_lookup_bdev() wrapper was updated to emulate
the behavior of the new lookup_bdev() for all supported kernels.

Signed-off-by: Brian Behlendorf <[email protected]>
The generic IO accounting functions have been removed in favor of the
bio_start_io_acct() and bio_end_io_acct() functions which provide a
better interface.  These new functions were introduced in the 5.8
kernels but it wasn't until the 5.11 kernel that the previous generic
IO accounting interfaces were removed.

This commit updates the blk_generic_*_io_acct() wrappers to provide
and interface similar to the updated kernel interface.  It's slightly
different because for older kernels we need to pass the request queue
as well as the bio.

Signed-off-by: Brian Behlendorf <[email protected]>
The bd_contains member was removed from the block_device structure.
Callers needing to determine if a vdev is a whole block device should
use the new bdev_whole() wrapper.  For older kernels we provide our
own bdev_whole() wrapper which relies on bd_contains for compatibility.

Signed-off-by: Brian Behlendorf <[email protected]>
Both revalidate_disk_size() and revalidate_disk() have been removed.
Functionally this isn't a problem because we only relied on these
functions to call zvol_revalidate_disk() for us and to perform any
additional handling which might be needed for that kernel version.
When neither are available we know there's no additional handling
needed and we can directly call zvol_revalidate_disk().

Signed-off-by: Brian Behlendorf <[email protected]>
As of 5.11 the blk_register_region() and blk_unregister_region()
functions have been retired. This isn't a problem since add_disk()
has implicitly allocated minor numbers for a very long time.

Signed-off-by: Brian Behlendorf <[email protected]>
@ckane
Copy link
Contributor

ckane commented Dec 24, 2020

I just built and am running off of your latest code now, too.

@rkitover
Copy link
Contributor

@behlendorf I don't know very much about kernel APIs, but looks great to me!

One minor change I'd make is collapse the 1-arg and 2-arg lookup_bdev() blocks into one block, since they are identical except for one line.

@behlendorf
Copy link
Contributor Author

@rkitover thanks for taking a look. I had that same thought about lookup_bdev() and made that change originally, however in the end I felt it actually hurt readability so I opted to split it up. I'd prefer to leave it as separate blocks even if it is a little redundant.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 28, 2020
behlendorf added a commit that referenced this pull request Dec 28, 2020
The lookup_bdev() function has been updated to require a dev_t
be passed as the second argument. This is actually pretty nice
since the major number stored in the dev_t was the only part we
were interested in. This allows to us avoid handling the bdev
entirely.  The vdev_lookup_bdev() wrapper was updated to emulate
the behavior of the new lookup_bdev() for all supported kernels.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
behlendorf added a commit that referenced this pull request Dec 28, 2020
The generic IO accounting functions have been removed in favor of the
bio_start_io_acct() and bio_end_io_acct() functions which provide a
better interface.  These new functions were introduced in the 5.8
kernels but it wasn't until the 5.11 kernel that the previous generic
IO accounting interfaces were removed.

This commit updates the blk_generic_*_io_acct() wrappers to provide
and interface similar to the updated kernel interface.  It's slightly
different because for older kernels we need to pass the request queue
as well as the bio.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
behlendorf added a commit that referenced this pull request Dec 28, 2020
The bd_contains member was removed from the block_device structure.
Callers needing to determine if a vdev is a whole block device should
use the new bdev_whole() wrapper.  For older kernels we provide our
own bdev_whole() wrapper which relies on bd_contains for compatibility.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
behlendorf added a commit that referenced this pull request Dec 28, 2020
Both revalidate_disk_size() and revalidate_disk() have been removed.
Functionally this isn't a problem because we only relied on these
functions to call zvol_revalidate_disk() for us and to perform any
additional handling which might be needed for that kernel version.
When neither are available we know there's no additional handling
needed and we can directly call zvol_revalidate_disk().

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
behlendorf added a commit that referenced this pull request Dec 28, 2020
As of 5.11 the blk_register_region() and blk_unregister_region()
functions have been retired. This isn't a problem since add_disk()
has implicitly allocated minor numbers for a very long time.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
@rkitover
Copy link
Contributor

Just wanted to say that I'm running this on gentoo with 5.11-rc1, no issues.

Thank you!

behlendorf added a commit that referenced this pull request Jan 5, 2021
Update the ZFS_LINUX_TEST_PROGRAM macro to always set the module
license.  As of the 5.11 kernel not setting a license has been
converted from a warning to an error.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
behlendorf added a commit that referenced this pull request Jan 5, 2021
The lookup_bdev() function has been updated to require a dev_t
be passed as the second argument. This is actually pretty nice
since the major number stored in the dev_t was the only part we
were interested in. This allows to us avoid handling the bdev
entirely.  The vdev_lookup_bdev() wrapper was updated to emulate
the behavior of the new lookup_bdev() for all supported kernels.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
behlendorf added a commit that referenced this pull request Jan 5, 2021
The generic IO accounting functions have been removed in favor of the
bio_start_io_acct() and bio_end_io_acct() functions which provide a
better interface.  These new functions were introduced in the 5.8
kernels but it wasn't until the 5.11 kernel that the previous generic
IO accounting interfaces were removed.

This commit updates the blk_generic_*_io_acct() wrappers to provide
and interface similar to the updated kernel interface.  It's slightly
different because for older kernels we need to pass the request queue
as well as the bio.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
behlendorf added a commit that referenced this pull request Jan 5, 2021
The bd_contains member was removed from the block_device structure.
Callers needing to determine if a vdev is a whole block device should
use the new bdev_whole() wrapper.  For older kernels we provide our
own bdev_whole() wrapper which relies on bd_contains for compatibility.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
behlendorf added a commit that referenced this pull request Jan 5, 2021
Both revalidate_disk_size() and revalidate_disk() have been removed.
Functionally this isn't a problem because we only relied on these
functions to call zvol_revalidate_disk() for us and to perform any
additional handling which might be needed for that kernel version.
When neither are available we know there's no additional handling
needed and we can directly call zvol_revalidate_disk().

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
behlendorf added a commit that referenced this pull request Jan 5, 2021
As of 5.11 the blk_register_region() and blk_unregister_region()
functions have been retired. This isn't a problem since add_disk()
has implicitly allocated minor numbers for a very long time.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes #11387
Closes #11390
@averyfreeman
Copy link

I am running into this issue building 2.0.4-1 against 5.11.8 kernel. Here's the tail of ./configure:

checking whether global_page_state enums are sane... yes
checking whether compile-time stack validation (objtool) is available... yes
checking whether STACK_FRAME_NON_STANDARD is defined... yes
checking whether PDE_DATA() is available... configure: error: 
        *** None of the expected "PDE_DATA" interfaces were detected.
        *** This may be because your kernel version is newer than what is
        *** supported, or you are using a patched custom kernel with
        *** incompatible modifications.
        ***
        *** ZFS Version: zfs-2.0.4-1
        *** Compatible Kernels: 3.10 - 5.11

Same message with clang-9 and gcc Debian 10.2.1 20210110 (I'm on bullseye/testing), and I also tried a clone of the master branch against 5.11.8.

I just tried with 5.11.6 kernel tarball and it passed ./configure and is building right now, so something regarding PDA_DATA interface changed between 5.11.6 and 5.11.8.

Do you think I should file a separate bug report?

@ckane
Copy link
Contributor

ckane commented Mar 23, 2021

I am running into this issue building 2.0.4-1 against 5.11.8 kernel. Here's the tail of ./configure:

checking whether global_page_state enums are sane... yes
checking whether compile-time stack validation (objtool) is available... yes
checking whether STACK_FRAME_NON_STANDARD is defined... yes
checking whether PDE_DATA() is available... configure: error: 
        *** None of the expected "PDE_DATA" interfaces were detected.
        *** This may be because your kernel version is newer than what is
        *** supported, or you are using a patched custom kernel with
        *** incompatible modifications.
        ***
        *** ZFS Version: zfs-2.0.4-1
        *** Compatible Kernels: 3.10 - 5.11

Same message with clang-9 and gcc Debian 10.2.1 20210110 (I'm on bullseye/testing), and I also tried a clone of the master branch against 5.11.8.

I just tried with 5.11.6 kernel tarball and it passed ./configure and is building right now, so something regarding PDA_DATA interface changed between 5.11.6 and 5.11.8.

Do you think I should file a separate bug report?

Yes, though first try building against the master branch to verify that this isn't already fixed in the next release, and try to include any relevant errors from configure that might have been spat into ./build/build.log* files.

I can verify that master was building against Arch's 5.11.7 kernel for me, and also builds against Arch's 5.11.8-arch1-1 on my system, too.

@rincebrain
Copy link
Contributor

I also can't reproduce this with 5.11.8 on Debian bullseye (generated with make olddefconfig against config-5.10.0-4-amd64), so you should probably include your kernel .config somewhere along with config.log when you post a bug report.

@averyfreeman
Copy link

averyfreeman commented Mar 24, 2021 via email

jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Update the ZFS_LINUX_TEST_PROGRAM macro to always set the module
license.  As of the 5.11 kernel not setting a license has been
converted from a warning to an error.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
The lookup_bdev() function has been updated to require a dev_t
be passed as the second argument. This is actually pretty nice
since the major number stored in the dev_t was the only part we
were interested in. This allows to us avoid handling the bdev
entirely.  The vdev_lookup_bdev() wrapper was updated to emulate
the behavior of the new lookup_bdev() for all supported kernels.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
The generic IO accounting functions have been removed in favor of the
bio_start_io_acct() and bio_end_io_acct() functions which provide a
better interface.  These new functions were introduced in the 5.8
kernels but it wasn't until the 5.11 kernel that the previous generic
IO accounting interfaces were removed.

This commit updates the blk_generic_*_io_acct() wrappers to provide
and interface similar to the updated kernel interface.  It's slightly
different because for older kernels we need to pass the request queue
as well as the bio.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
The bd_contains member was removed from the block_device structure.
Callers needing to determine if a vdev is a whole block device should
use the new bdev_whole() wrapper.  For older kernels we provide our
own bdev_whole() wrapper which relies on bd_contains for compatibility.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
Both revalidate_disk_size() and revalidate_disk() have been removed.
Functionally this isn't a problem because we only relied on these
functions to call zvol_revalidate_disk() for us and to perform any
additional handling which might be needed for that kernel version.
When neither are available we know there's no additional handling
needed and we can directly call zvol_revalidate_disk().

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
jsai20 pushed a commit to jsai20/zfs that referenced this pull request Mar 30, 2021
As of 5.11 the blk_register_region() and blk_unregister_region()
functions have been retired. This isn't a problem since add_disk()
has implicitly allocated minor numbers for a very long time.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
@behlendorf behlendorf deleted the linux-5.11-compat branch April 19, 2021 19:20
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
Update the ZFS_LINUX_TEST_PROGRAM macro to always set the module
license.  As of the 5.11 kernel not setting a license has been
converted from a warning to an error.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
The lookup_bdev() function has been updated to require a dev_t
be passed as the second argument. This is actually pretty nice
since the major number stored in the dev_t was the only part we
were interested in. This allows to us avoid handling the bdev
entirely.  The vdev_lookup_bdev() wrapper was updated to emulate
the behavior of the new lookup_bdev() for all supported kernels.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
The generic IO accounting functions have been removed in favor of the
bio_start_io_acct() and bio_end_io_acct() functions which provide a
better interface.  These new functions were introduced in the 5.8
kernels but it wasn't until the 5.11 kernel that the previous generic
IO accounting interfaces were removed.

This commit updates the blk_generic_*_io_acct() wrappers to provide
and interface similar to the updated kernel interface.  It's slightly
different because for older kernels we need to pass the request queue
as well as the bio.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
The bd_contains member was removed from the block_device structure.
Callers needing to determine if a vdev is a whole block device should
use the new bdev_whole() wrapper.  For older kernels we provide our
own bdev_whole() wrapper which relies on bd_contains for compatibility.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
Both revalidate_disk_size() and revalidate_disk() have been removed.
Functionally this isn't a problem because we only relied on these
functions to call zvol_revalidate_disk() for us and to perform any
additional handling which might be needed for that kernel version.
When neither are available we know there's no additional handling
needed and we can directly call zvol_revalidate_disk().

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
sempervictus pushed a commit to sempervictus/zfs that referenced this pull request May 31, 2021
As of 5.11 the blk_register_region() and blk_unregister_region()
functions have been retired. This isn't a problem since add_disk()
has implicitly allocated minor numbers for a very long time.

Reviewed-by: Rafael Kitover <[email protected]>
Reviewed-by: Coleman Kane <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#11387
Closes openzfs#11390
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested) Type: Building Indicates an issue related to building binaries
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants