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

zfs-2.2.7 patchset #16720

Open
wants to merge 214 commits into
base: zfs-2.2-release
Choose a base branch
from

Conversation

tonyhutter
Copy link
Contributor

Motivation and Context

  • Fully support 6.11 kernel
  • Add QEMU github runners for testing

Description

Proposed zfs-2.2.7 patchset. Support 6.11 kernel and github QEMU runners.

How Has This Been Tested?

Runners will test

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:

amotin and others added 30 commits November 4, 2024 10:34
 - Use the macros in few places it was missed.
 - Reduce scope of DB_DNODE_ENTER/EXIT() and inline some DB_DNODE()
uses to make it more obvious what exactly is protected there and
make unprotected accesses by mistake more difficult.
 - Make use of zrl_owner().

Reviewed-by: Rob Wing <[email protected]
Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Allan Jude <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by:	Alexander Motin <[email protected]>
Sponsored by:	iXsystems, Inc.
Closes openzfs#16374
ZIL log record structs (lr_XX_t) are frequently allocated with extra
space after the struct to carry variable-sized "payload" items.

Linux 6.10+ compiled with CONFIG_FORTIFY_SOURCE has been doing runtime
bounds checking on memcpy() calls. Because these types had no indicator
that they might use more space than their simple definition,
__fortify_memcpy_chk will frequently complain about overruns eg:

    memcpy: detected field-spanning write (size 7) of single field
        "lr + 1" at zfs_log.c:425 (size 0)
    memcpy: detected field-spanning write (size 9) of single field
        "(char *)(lr + 1)" at zfs_log.c:593 (size 0)
    memcpy: detected field-spanning write (size 4) of single field
        "(char *)(lr + 1) + snamesize" at zfs_log.c:594 (size 0)
    memcpy: detected field-spanning write (size 7) of single field
        "lr + 1" at zfs_log.c:425 (size 0)
    memcpy: detected field-spanning write (size 9) of single field
        "(char *)(lr + 1)" at zfs_log.c:593 (size 0)
    memcpy: detected field-spanning write (size 4) of single field
        "(char *)(lr + 1) + snamesize" at zfs_log.c:594 (size 0)
    memcpy: detected field-spanning write (size 7) of single field
        "lr + 1" at zfs_log.c:425 (size 0)
    memcpy: detected field-spanning write (size 9) of single field
        "(char *)(lr + 1)" at zfs_log.c:593 (size 0)
    memcpy: detected field-spanning write (size 4) of single field
        "(char *)(lr + 1) + snamesize" at zfs_log.c:594 (size 0)

To fix this, this commit adds flex array fields to all lr_XX_t structs
that require them, and then uses those fields to access that
end-of-struct area rather than more complicated casts and pointer
addition.

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16501
Closes openzfs#16539
This sets RHEL8's base kernel[1] as the floor, and includes the oldest
kernel.org LTS (4.19).

1. https://access.redhat.com/articles/3078#RHEL8

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16479
Update the META file to reflect compatibility with the 6.11 kernel.

Reviewed-by: Rob Norris <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16586
Last commit should fix the underlying problem, so these should be
passing reliably again.

Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16364
Optionally turn off disk's enclosure slot if an I/O is hung
triggering the deadman.

It's possible for outstanding I/O to a misbehaving SCSI disk to
neither promptly complete or return an error.  This can occur due
to retry and recovery actions taken by the SCSI layer, driver, or
disk.  When it occurs the pool will be unresponsive even though
there may be sufficient redundancy configured to proceeded without
this single disk.

When a hung I/O is detected by the kmods it will be posted as a
deadman event.  By default an I/O is considered to be hung after
5 minutes.  This value can be changed with the zfs_deadman_ziotime_ms
module parameter.  If ZED_POWER_OFF_ENCLOSURE_SLOT_ON_DEADMAN is set
the disk's enclosure slot will be powered off causing the outstanding
I/O to fail.  The ZED will then handle this like a normal disk failure.
By default ZED_POWER_OFF_ENCLOSURE_SLOT_ON_DEADMAN is not set.

As part of this change `zfs_deadman_events_per_second` is added
to control the ratelimitting of deadman events independantly of
delay events.  In practice, a single deadman event is sufficient
and more aren't particularly useful.

Alphabetize the zfs_deadman_* entries in zfs.4.

Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16226
Some libc's like uClibc lag the proper definition of SEEK_DATA
and SEEK_HOLE. Since we have only two files in ZTS which use
these definitons, let's define them by hand:

```
#ifndef SEEK_DATA
#define SEEK_DATA 3
#endif
#ifndef SEEK_HOLE
#define SEEK_HOLE 4
#endif
```

There should be no failures, because:
- FreeBSD has support for SEEK_DATA/SEEK_HOLE since FreeBSD 8
- Linux has it since Linux 3.1
- the libc will submit the parameters unchanged to the kernel

Signed-off-by: Tino Reichardt <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: Allan Jude <[email protected]>
Signed-off-by: Mateusz Piotrowski <[email protected]>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#16248
Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: Allan Jude <[email protected]>
Signed-off-by: Mateusz Piotrowski <[email protected]>
Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Closes openzfs#16248
Reviewed-by: George Melikov <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Ameer Hamza <[email protected]>
Closes openzfs#16432
The report generator expects the log to be clean and tidy UTF-8. That
can be a problem if you use some of the verbose/debug test runner
options, which sends all sorts of weird output from arbitrary programs
to the log.

This just makes Python a little more relaxed about such things. It
shouldn't matter in practice, as those lines didn't match the test
result regex anyway, and are discarded immediately.


Sponsored-by: https://despairlabs.com/sponsor/

Signed-off-by: Rob Norris <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
The test needs some adjusting within the timings.

Reviewed by: Brian Behlendorf <[email protected]>
Signed-off-by: Tony Hutter <[email protected]>
Co-authored-by: Tino Reichardt <[email protected]>
Closes openzfs#16537
On load the test needs sometimes a bit more time then just one second.
Doubling the time will help on the QEMU based testings.

Reviewed by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Closes openzfs#16537
This commit adds functional tests for these systems:
- AlmaLinux 8, AlmaLinux 9, ArchLinux
- CentOS Stream 9, Fedora 39, Fedora 40
- Debian 11, Debian 12
- FreeBSD 13, FreeBSD 14, FreeBSD 15
- Ubuntu 20.04, Ubuntu 22.04, Ubuntu 24.04

- enabled by default:
 - AlmaLinux 8, AlmaLinux 9
 - Debian 11, Debian 12
 - Fedora 39, Fedora 40
 - FreeBSD 13, FreeBSD 14

Workflow for each operating system:
- install qemu on the github runner
- download current cloud image of operating system
- start and init that image via cloud-init
- install dependencies and poweroff system
- start system and build openzfs and then poweroff again
- clone build system and start 2 instances of it
- run functional testings and complete in around 3h
- when tests are done, do some logfile preparing
- show detailed results for each system
- in the end, generate the job summary

Real-world benefits from this PR:

1. The github runner scripts are in the zfs repo itself. That means
   you can just open a PR against zfs, like "Add Fedora 41 tester", and
   see the results directly in the PR. ZFS admins no longer need
   manually to login to the buildbot server to update the buildbot config
   with new version of Fedora/Almalinux.

2. Github runners allow you to run the entire test suite against your
   private branch before submitting a formal PR to openzfs. Just open a
   PR against your private zfs repo, and the exact same
   Fedora/Alma/FreeBSD runners will fire up and run ZTS. This can be
   useful if you want to iterate on a ZTS change before submitting a
   formal PR.

3. buildbot is incredibly cumbersome. Our buildbot config files alone
   are ~1500 lines (not including any build/setup scripts)!
   It's a huge pain to setup.

4. We're running the super ancient buildbot 0.8.12. It's so ancient
   it requires python2. We actually have to build python2 from source
   for almalinux9 just to get it to run. Ugrading to a more modern
   buildbot is a huge undertaking, and the UI on the newer versions is
   worse.

5. Buildbot uses EC2 instances. EC2 is a pain because:
   * It costs money
   * They throttle IOPS and CPU usage, leading to mysterious,
   * hard-to-diagnose, failures and timeouts in ZTS.
   * EC2 is high maintenance. We have to setup security groups, SSH
   * keys, networking, users, etc, in AWS and it's a pain. We also
   * have to periodically go in an kill zombie EC2 instances that
   * buildbot is unable to kill off.

6. Buildbot doesn't always handle failures well. One of the things we
   saw in the past was the FreeBSD builders would often die, and each
   builder death would take up a "slot" in buildbot. So we would
   periodically have to restart buildbot via a cron job to get the slots
   back.

7. This PR divides up the ZTS test list into two parts, launches two
   VMs, and on each VM runs half the test suite. The test results are
   then merged and shown in the sumary page. So we're basically
   parallelizing ZTS on the same github runner. This leads to lower
   overall ZTS runtimes (2.5-3 hours vs 4+ hours on buildbot), and one
   unified set of results per runner, which is nice.

8. Since the tests are running on a VM, we have much more control over
   what happens. We can capture the serial console output even if the
   test completely brings down the VM. In the future, we could also
   restart the test on the VM where it left off, so that if a single test
   panics the VM, we can just restart it and run the remaining ZTS tests
   (this functionaly is not yet implemented though, just an idea).

9. Using the runners, users can manually kill or restart a test run
   via the github IU. That really isn't possible with buildbot unless
   you're an admin.

10. Anecdotally, the tests seem to be more stable and constant under
    the QEMU runners.

Reviewed by: Brian Behlendorf <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Signed-off-by: Tony Hutter <[email protected]>
Closes openzfs#16537
On larger files this should improve the speed.

Sample values of my system:

[mcmilk@xz]$ time dd if=/dev/zero bs=128k count=1k | sha256sum
254bcc3fc4f27172636df4bf32de9f107f620d559b20d760197e452b97453917  -
real    0m1,050s
user    0m0,985s
sys     0m0,153s

[mcmilk@xz]$ time dd if=/dev/zero bs=128k count=1k | openssl sha256 -r
254bcc3fc4f27172636df4bf32de9f107f620d559b20d760197e452b97453917 *stdin
real    0m0,254s
user    0m0,206s
sys     0m0,160s

I think cli_root/zdb/zdb_backup.ksh runs also an FreeBSD and I needed to
include the sysutils/coreutils package for the FreeBSD tests within the
QEMU patchset.

This could be reverted, when this pull request gets upstream

Reviewed-by: George Melikov <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Closes openzfs#16543
Fix that error: "cat /tmp/failed.txt: No such file or directory"

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Closes openzfs#16549
This commit changes the workflow of the github actions.

- Ubuntu 20.04, 22.04, 24.04 will be tested via QEMU now
- remove unused scripts of this commit: b7bc334
- re-add the zloop standalone testings via zloop.yml

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Closes openzfs#16549
In zpool_create.shlib, check_feature_set iterates over all features
mentioned in provided compatibility file to check if only those
features are enabled on the pool.

This commit fixes skipping over comment lines correctly. Otherwise,
the test case fails as comment lines are also treated as feature names
by check_feature_set function.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Umer Saleem <[email protected]>
Closes openzfs#15909
The qemu-9-summary-page.sh script reads the file env.txt in the
first lines. When the module didn't build, this file was not copied
into the tarfile - causing the scipt to abort.

Fix: copy needed files into the tarfile in case of module build
failures. The fix ignores also empty tarfiles in future.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Tino Reichardt <[email protected]>
Closes openzfs#16555
There is no longer be a need for the ci_reason exception with
the update CI GitHub Actions infrastruture.  Retire it.

Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16553
All supported Linux kernels, 4.18 and newer, provide O_TMPFILE.

Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16553
The following tests have been observed to occasionally fail when
running under the CI.  Updated our exceptions list to track them.

Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16553
Switch from v2 to v3 CodeQL Actions.  The v2 actions will no longer
be supported as of Dec '24 so we need to move to v3.  According to
the release notes they should be functionally equivalent.

    Note that the only difference between v2 and v3 of the CodeQL
    Action is the node version they support, ... For example 3.22.11
    was the first v3 release and is functionally identical to 2.22.11.

https://github.com/github/codeql-action/blob/main/CHANGELOG.md

Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16560
Update the CONTRIBUTING.md documentation to refer to the GitHub Actions
workflows which have replaced the buildbot infrastructure.

Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16561
For checkstyle, zloop, zfs-qemu, and codeql workflows cancel
in-progress jobs when the PR is updated.

Relevant GitHub Actions documentation:

  The following concurrency group cancels in-progress jobs or run
  on pull_request events only; if github.head_ref is undefined, the
  concurrency group will fallback to the run ID, which is guaranteed
  to be both unique and defined for the run.

https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#example-using-a-fallback-value

Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16562
The commit uses heuristics to determine whether a PR is behavioral:

It runs "quick" CI (i.e., only use sanity.run on fewer OSes)
if (explicitly requested by user):
- the *last* commit message contains a line 'ZFS-CI-Type: quick',
or if (by heuristics):
- the files changed are not in the list of specified directory, and
- all commit messages does not contain 'ZFS-CI-Type: full'.

It runs "full" CI otherwise.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Signed-off-by: Shengqi Chen <[email protected]>
Closes openzfs#16564
Update the test case to freeze the pool then export it to better
simulate a hard failure.  This is preferable to copying the vdev
while the pool's imported since with a copy we're not guaranteed
the on-disk state will be consistent.  That can in turn result
in a pool import failure and a spurious test failure.

Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16570
On failure attempt to include the most relevant portions of the
ztest logs in the CI output.  This full logs are still available
for download but often a backtrace and the last output is enough.

Install libunwind to improve the odds of a useful backtrace.

Reviewed-by: Tino Reichardt <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16573
Lower the minimum number of expected deadman events from 4 to 3.  All
that is strictly required is a single event to consider the test a
pass.  However, since I've never seen a count of less than 3 reported
by the CI that should be sufficient.

Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16575
Update the test case to freeze the pool then export it to better
simulate a hard failure.  This is preferable to copying the vdev
while the pool's imported since with a copy we're not guaranteed
the on-disk state will be consistent.  That can in turn result
in a pool import failure and a spurious test failure.

Reviewed-by: Tino Reichardt <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Closes openzfs#16578
robn and others added 16 commits November 15, 2024 10:15
This adds zfs_valstr, a collection of pretty printers for bitfields and
enums. These are useful in debugging, logging and other display contexts
where raw values are difficult for the untrained (or even trained!) eye
to decipher.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>
The Linux arc_os.c carries userspace and kernel code, with very little
overlap between the two. This lifts the userspace parts out into a
separate arc_os.c for libzpool and removes it from the Linux side.

Reviewed by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16492
The no-op is fine for both.

Reviewed by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16492
It does nothing in userspace anyway.

Reviewed by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16492
The no-op is fine for both.

Reviewed by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16492
This includes the last 12.x release (now EOL) and 13.0 development
versions (<1300139).

Sponsored-by: https://despairlabs.com/sponsor/

Signed-off-by: Rob Norris <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
For now, userspace has no znode implementation. Some of the property and
path handling code is used there though and is the same on all
platforms, so we only need a single copy of it.

Reviewed by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16492
FreeBSD was using fprintf(), which might not be signal-safe. Meanwhile,
Linux's locking did not cover the header output. This two quirks are
unrelated, but both have the same response: be like the other one. So
with this commit, both functions are the same except for the names of
their lock and list variables.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16181
And, make the output fd an arg to zfs_dbgmsg_print(). This is a change
in behaviour, but keeps it consistent with where crash traces go, and
it's easy to argue this is what we want anyway; this is information
about the task, not the actual output of the task.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16181
Just nice and simple, with room to grow.

Reviewed by: Brian Behlendorf <[email protected]>
Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#16492
Add vdev_check_boot_reserve() to vdev.h for zfs-2.2.7 compatibility.

Signed-off-by: Tony Hutter <[email protected]>
torvalds/linux@b2e7456b5c25 makes kmem_cache_create() a macro, which
gets in the way of our our own redefinition, so we undef the macro first
for our own clients. This follows what we did for kmem_cache_alloc(),
see e951dba.

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16582
See torvalds/linux@a2b80ce87a87. It claims the task arg is always
`current`, and so it is with us, so this is a safe change to make. The
only spanner is that we also support the older pre-5.17 3-arg
dequeue_signal() which had different meaning, so we have to check the
types to get the right one.

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16582
torvalds/linux@641bb4394f40 asserts that this is a static flag, not
intended to be variable per-file, so it moves it to
file_operations instead. We just change our check to follow.

No configure check is necessary because FOP_UNSIGNED_OFFSET didn't exist
before this commit, and FMODE_UNSIGNED_OFFSET flag is removed in the
same commit, so there's no chance of a conflict.

It's not clear to me that we need this check at all, as we never set
this flag on our own files, and I can't see any way that our llseek
handler could recieve a file from another filesystem. But, the whole
zpl_llseek() has a number of opportunities for pleasing cleanup that are
nothing to do with this change, so I'll leave that for a future change.

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16582
linux/torvalds@11068e0b64cb removes it, suggesting this was a always
there as a helper to handle concurrent seeks, which all filesystems now
handle themselves if necessary.

Without looking into the mechanism, I can imagine how it might have been
used, but we have always set it to zero and never read from it,
presumably because we've always tracked per-caller position through the
znode anyway. So I don't see how there can be any functional change for
us by removing it. I've stayed conservative though and left it in for
older kernels, since its clearly not hurting anything there.

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16582
torvalds/linux@09022bc196d2 removes the flag, and the corresponding
SetPageError() and ClearPageError() macros, with no replacement offered.

Going back through the upstream history, use of this flag has been
gradually removed over the last year as part of the long tail of
converting everything to folios. Interesting tidbit comments from
torvalds/linux@29e9412b250e and torvalds/linux@420e05d0de18 suggest that
this flag has not been used meaningfully since page writeback failures
started being recorded in errseq_t instead (the whole "fsyncgate" thing,
~2017, around torvalds/linux@8ed1e46aaf1b).

Given that, it's possible that since perhaps Linux 4.13 we haven't been
getting anything by setting the flag. I don't know if that's true and/or
if there's something we should be doing instead, but my gut feel is that
its probably fine we only use the page cache as a proxy to allow mmap()
to work, rather than backing IO with it.

As such, I'm expecting that removing this will do no harm, but I'm
leaving it in for older kernels to maintain status quo, and if there is
an overall better way, that is left for a future change.

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16582
Copy link

@3dRikal 3dRikal left a comment

Choose a reason for hiding this comment

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

No issues here

@mabod
Copy link

mabod commented Nov 18, 2024

I have this PR running since 3 days on kernel 6.6.x on Endeavouros without any issue.

@tonyhutter
Copy link
Contributor Author

tonyhutter commented Nov 18, 2024

Looks like module load isn't working on FreeBSD:

Test: /usr/local/share/zfs/zfs-tests/tests/functional/acl/off/setup (run as root) [00:00] [FAIL]
19:28:11.98 SUCCESS: add_group zfsgrp
19:28:12.00 SUCCESS: add_user zfsgrp staff1
19:28:12.02 SUCCESS: add_user zfsgrp staff2
19:28:12.02 NOTE: begin default_setup_noexit
19:28:12.14 Failed to load openzfs module: Exec format error
19:28:12.14 ERROR: zpool create -f testpool md0 exited 1
...

19:35:34.34 linker_load_file: /boot/modules/openzfs.ko - unsupported file type
19:35:34.34 link_elf_obj: symbol zfs_znode_update_vfs undefined
19:35:34.34 linker_load_file: /boot/modules/openzfs.ko - unsupported file type
19:35:34.34 link_elf_obj: symbol zfs_znode_update_vfs undefined
19:35:34.34 linker_load_file: /boot/modules/openzfs.ko - unsupported file type
19:35:34.34 link_elf_obj: symbol zfs_znode_update_vfs undefined
19:35:34.34 linker_load_file: /boot/modules/openzfs.ko - unsupported file type

I'm looking into it

rincebrain and others added 4 commits November 18, 2024 11:44
Being able to print custom debug information on assert trip
seems useful.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Paul Dagnelie <[email protected]>
Signed-off-by: Rich Ercolani <[email protected]>
Closes openzfs#15792
The FreeBSD linux/compiler.h in OpenZFS was copied from a very old
version of FreeBSD's linuxkpi's linux/compiler.h. There's no need for
this duplication. Use FreeBSD's linuxkpi version instead, and provide
zfs_fallthrough to augment it (it's all that's needed). Use #pragma once
to avoid naming issues for guard variables. Since this is a complete
rewrite, use my copyright here (the original code in FreeBSD still
credits everybody). This works back at least to FreeBSD 12.4, which
is not out of support, and all newer releases.

Remove extra copies of macros that were defined elsewhere, but are now
properly defined in LinuxKPI so are redundant.

Sponsored-by: Netflix
Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Warner Losh <[email protected]>
Closes openzfs#16650
With FreeBSD's switch to git the $FreeBSD$ string is no longer expanded
and they have mostly been removed upstream.  Stop using __FBSDID and
remove the no-longer needed sys/cdefs.h includes.

Reviewed-by: Alexander Motin <[email protected]>
Signed-off-by: Brooks Davis <[email protected]>
Closes openzfs#15527
These members have directly references to the global variables
exposed by the kernel. They are not going to be changed by this
kernel module.

Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Zhenlei Huang <[email protected]>
Closes openzfs#16210
@robn
Copy link
Member

robn commented Nov 19, 2024

@tonyhutter You want 80645d6.

robn and others added 2 commits November 20, 2024 08:55
I accidentally removed this in c22d56e, and didn't notice because it
doesn't fail the build, but does fail to load into the kernel because it
can't link it.

Sponsored-by: https://despairlabs.com/sponsor/
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: George Melikov <[email protected]>
Signed-off-by: Rob Norris <[email protected]>
Closes openzfs#16554
META file and changelog updated.

Signed-off-by: Tony Hutter <[email protected]>
@tonyhutter
Copy link
Contributor Author

@robn thanks, I included it in my latest push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.