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 6.9 compatibility #16033

Closed
wants to merge 2 commits into from
Closed

Linux 6.9 compatibility #16033

wants to merge 2 commits into from

Conversation

robn
Copy link
Member

@robn robn commented Mar 27, 2024

Motivation and Context

Typical churn, thankfully a little smaller than most releases.

Closes #16027.

Description

See individual commits.

How Has This Been Tested?

For each of the following kernels:

  • 6.9.0-rc1
  • 6.8.0-rc3
  • 6.7.9
  • 6.1.38
  • 5.10.170
  • 4.14.336
  • 4.4.302

Follwing tests succeded:

  • basic sanity check: load module, create pool, short fio run, export, import, scrub, export, unload module
  • ZTS: zvol
  • ZTS: zpool_create
  • ZTS: zpool_reopen
  • ZTS: block_cloning

Given that that the original 6.8 patch series fell short in a way we could have caught (#15931), and since people apparently can't help but run unsupported combinations of Linux+OpenZFS (#15986), I'd at least like to get a full ZTS run against this PR before merge. However right now I don't have anything that can run the whole test suite and also can easily accommodate a sufficiently recent kernel. If someone else can run it, that would be fantastic, otherwise I'll try and get something going in the next week or two.

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:

@darkbasic
Copy link

I'd at least like to get a full ZTS run against this PR before merge

Don't existing GitHub Actions already run the test suite? If so it would be a matter of adding an additional step to add the kernel ppa in the docker container build file to automate the process for the future.

@robn
Copy link
Member Author

robn commented Mar 27, 2024

@darkbasic they do, but against specific a specific set of distributions. I don't know much about how they're set up. I suspect what you're suggesting is not outrageously complicated, but not flipping a switch either. Support for this would likely be very welcome!

Copy link
Contributor

@tonyhutter tonyhutter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that master does not build against the 6.9 Ubuntu nightly kernel, but it does build with this PR.

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. But I agree it'd be good to get a full test run with the latest kernel.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Mar 29, 2024
@darkbasic
Copy link

On Monday I will try to update the CI to add a manual trigger for development kernels.
Feel free to test it manually in the meantime because CI stuff is slow as hell to run and if I encounter any issues I might not have the time to complete it.

robn added 2 commits April 2, 2024 13:19
bdev_open_by_path() is replaced by bdev_file_open_by_path(), which
returns a plain old struct file*. Release function is gone entirely; the
regular file release function fput() will take care of the bdev
specifics.

Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16027
There's an extra nullable arg for queue limits. Detect it, and set it to
NULL. Similar change for blk_mq_alloc_disk(), now three args, same
treatment.

Error return now has error encoded in the return, so detect with
IS_ERR() and explicitly NULL our own return.

Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16027
@darkbasic
Copy link

I gave it a try, starting by forking the repo and setting up ci on my fork. I've noticed that dotnet-sdk from the microsoft repo fails to download most of the times: E: Failed to fetch https://packages.microsoft.com/ubuntu/20.04/prod/pool/main/d/dotnet-sdk-8.0/dotnet-sdk-8.0_8.0.203-1_amd64.deb Connection failed [IP: 13.107.246.69 443]

https://github.com/darkbasic/zfs/actions/runs/8492015289/job/23264864791

Restarting the failed job sometimes fix it, it's a roulette at this point.

Do you have any idea why this happens? Does it happen on this repo as well? It's a bit hard to work on if I can't get the existing infrastructure to work consistently to begin with.
Also I've noticed that many existing tests are already failing for this PR: is it expected?

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Apr 3, 2024
@behlendorf
Copy link
Contributor

Do you have any idea why this happens? Does it happen on this repo as well?

I've seen bouts of this before where there are transient issues with some repos. It's annoying, but outside our control. Usually it doesn't last too long.

You may see a couple of spurious CI test failures but they shouldn't fail reliably. The failures in this PR are for know issues. This looks good to me so I've gone ahead and merged it.

@behlendorf behlendorf closed this in e3120f7 Apr 3, 2024
behlendorf pushed a commit that referenced this pull request Apr 3, 2024
There's an extra nullable arg for queue limits. Detect it, and set it to
NULL. Similar change for blk_mq_alloc_disk(), now three args, same
treatment.

Error return now has error encoded in the return, so detect with
IS_ERR() and explicitly NULL our own return.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes #16027
Closes #16033
@robn
Copy link
Member Author

robn commented Apr 3, 2024

@behlendorf thanks. I'm getting a usable any-kernel-I-want ZTS runner vm up over here, so I'm hoping to have a full test run on 6.9 by the weekend. If anything shakes out you'll hear from me!

tonyhutter pushed a commit that referenced this pull request May 2, 2024
bdev_open_by_path() is replaced by bdev_file_open_by_path(), which
returns a plain old struct file*. Release function is gone entirely; the
regular file release function fput() will take care of the bdev
specifics.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes #16027
Closes #16033
tonyhutter pushed a commit that referenced this pull request May 2, 2024
There's an extra nullable arg for queue limits. Detect it, and set it to
NULL. Similar change for blk_mq_alloc_disk(), now three args, same
treatment.

Error return now has error encoded in the return, so detect with
IS_ERR() and explicitly NULL our own return.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes #16027
Closes #16033
@robn robn mentioned this pull request Jun 21, 2024
13 tasks
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
bdev_open_by_path() is replaced by bdev_file_open_by_path(), which
returns a plain old struct file*. Release function is gone entirely; the
regular file release function fput() will take care of the bdev
specifics.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16027
Closes openzfs#16033
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
There's an extra nullable arg for queue limits. Detect it, and set it to
NULL. Similar change for blk_mq_alloc_disk(), now three args, same
treatment.

Error return now has error encoded in the return, so detect with
IS_ERR() and explicitly NULL our own return.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16027
Closes openzfs#16033
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)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linux 6.9 compat: bdev_open_by_path replaced by bdev_file_open_by_path
4 participants