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.

Sign up for GitHub

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

Jump to bottom

ZFS 0.8.6 Linux 5.10 sendfile backport #11376

Closed
wants to merge 9 commits into from
Closed

ZFS 0.8.6 Linux 5.10 sendfile backport #11376

wants to merge 9 commits into from

Conversation

veggiemike
Copy link
Contributor

Motivation and Context

This is a backport of pull request #11351 to ZFS 0.8.6.

Description

Cherry-picked 1c2358c on top of my zfs-0.8.6-fixes branch from pull request #11348.

Given that backporting to 0.8.6 means going back to not caring (much) about the common/os split, I've applied ALL the zvol.c, zvol_os.c, zfs_vnops.c, and zfs_vnops_os.c changes to the original files in module/zfs/. Here's a summary of the other conflicts in the commit when cherry-picked onto 0.8.6:

  • I left zpl_fallocate_common() in zpl.h and in zpl_file.c because it's still needed. Looks like on master this has either been removed or split up and moved somewhere else. I left it alone.

  • uio.uio_limit was set to MAXOFFSET_T in 0.8.6 in a couple hunks that failed, but the hunks were removing the entire uio struct, so it seemed harmless.

  • Looks like master uses ITOZ() in a couple places that broke hunks, but again, it was in code being removed.

  • zfs_write_simple() doesn't exist in 0.8.6. I copied the entire function into zfs_vnops.c and declared it in zfs_vnops.h... Seems to work OK with a little tweaking.

  • Moved zfs_uio.c back into zcommon.

After that, there's a series of small commits fixing compilation errors I'd induced.

How Has This Been Tested?

Code now compiles against 5.10. Basic testing seems good. I can import my pool, play with ZVOLs, clone and compile the Linux kernel. I haven't seen any problems, but I also have not specifically tested sendfile(2).

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:

veggiemike and others added 9 commits December 15, 2020 16:03
@veggiemike
Linux 5.10 compat: don't use revalidate_disk()
28c76d9 
Kernel 5.10 removed the revalidate_disk() helper function, which called a
device's revalidate_disk callback followed by revalidate_disk_size().

See mainline commit de09077c block: remove revalidate_disk()

Signed-off-by: Michael D Labriola <michael.d.labriola@gmail.com>
@ckane @veggiemike
Linux 5.10 compat: add configure check for revalidate_disk_size()
cd1ac1f 
This commit is a partial cherry-pick of 59b6872 from the master branch.
I've taken the configure portion verbatim, but the conditional code is
specifically for zfs-0.8.6.

Signed-off-by: Michael D Labriola <michael.d.labriola@gmail.com>
@veggiemike
zvol: remove forward-declaration of zvol_revalidate_disk()
8e353b9 
This commit avoids needing to forward-declare zvol_revalidate_disk() by
calling it via the disk->fops->revalidate_disk function pointer.

Signed-off-by: Michael D Labriola <michael.d.labriola@gmail.com>
@ckane @veggiemike
Linux 5.10 compat: percpu_ref added data member
af1e221 
Kernel commit 2b0d3d3e4fcfb brought in some changes to the struct
percpu_ref structure that moves most of its fields into a member
struct named "data" of type struct percpu_ref_data. This includes
the "count" member which is updated by vdev_blkg_tryget(), so update
this function to chase the API change, and detect it via configure.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Coleman Kane <ckane@colemankane.org>
Closes openzfs#11085
@behlendorf @veggiemike
Linux 5.10 compat: use iov_iter in uio structure
1ed6186 
This is a cherry-pick of 1c2358c from
the master branch, backported to 0.8.6.  Obviously, some file renaming
and file splitting had to be undone.  Also, the entire
zfs_write_simple() function was pulled in from master.

-MDL

As of the 5.10 kernel the generic splice compatibility code has been
removed.  All filesystems are now responsible for registering a
->splice_read and ->splice_write callback to support this operation.

The good news is the VFS provided generic_file_splice_read() and
iter_file_splice_write() callbacks can be used provided the ->iter_read
and ->iter_write callback support pipes.  However, this is currently
not the case and only iovecs and bvecs (not pipes) are ever attached
to the uio structure.

This commit changes that by allowing full iov_iter structures to be
attached to uios.  Ever since the 4.9 kernel the iov_iter structure
has supported iovecs, kvecs, bvevs, and pipes so it's desirable to
pass the entire thing when possible.  In conjunction with this the
uio helper functions (i.e uiomove(), uiocopy(), etc) have been
updated to understand the new UIO_ITER type.

Note that using the kernel provided uio_iter interfaces allowed the
existing Linux specific uio handling code to be simplified.  When
there's no longer a need to support kernel's older than 4.9, then
it will be possible to remove the iovec and bvec members from the
uio structure and always use a uio_iter.  Until then we need to
maintain all of the existing types for older kernels.

Some additional refactoring and cleanup was included in this change:

- Added checks to configure to detect available iov_iter interfaces.
  Some are available all the way back to the 3.10 kernel and are used
  when available.  In particular, uio_prefaultpages() now always uses
  iov_iter_fault_in_readable() which is available for all supported
  kernels.

- The unused UIO_USERISPACE type has been removed.  It is no longer
  needed now that the uio_seg enum is platform specific.

- Moved zfs_uio.c from the zcommon.ko module to the Linux specific
  platform code for the zfs.ko module.  This gets it out of libzfs
  where it was never needed and keeps this Linux specific code out
  of the common sources.  (This was undone for 0.8.6)

- Removed unnecessary O_APPEND handling from zfs_iter_write(), this
  is redundant and O_APPEND is already handled in zfs_write();

Reviewed-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes openzfs#11351
@veggiemike
include: fix missing includes to get backported iov_iter code compiling
0a7f452 
This commit adds a couple extra include statements to get the new iov_iter
code backported via 1ed6186 compiling.

Signed-off-by: Michael D Labriola <michael.d.labriola@gmail.com>
@veggiemike
zfs: use zfs_write_simple() instead of zfs_write_common()
19f005c 
Backport commit 1ed6186 replaced zfs_write_common() w/ zfs_write_simple(),
which has slightly different usage.  This commit fixes one instance of
zfs_write_common() in zfs_replay.c.

Signed-off-by: Michael D Labriola <michael.d.labriola@gmail.com>
@veggiemike
zfs: can't use uio_resid() macro
2f2d8c3 
Backport commit 1ed6186 added a bunch of code using uio_resid() macro,
which hasn't been backported.  This commit fleshes those instances out to
uio.uio_resid to get things compiling.

Signed-off-by: Michael D Labriola <michael.d.labriola@gmail.com>
@veggiemike
zfs: zfs_read/write functions take inode not znode
8655dd1 
Backport commit 1ed6186 added a bunch of calls to zfs_read/write() passing
in a znode as 1st argument.  Back in 0.8, those functions still took inodes
so this commit removes a couple ITOZ() macros and adds a ZTOI() macro where
needed.

Signed-off-by: Michael D Labriola <michael.d.labriola@gmail.com>
@PrivatePuffin
Copy link
Contributor

This should be send to a (yet to be created) 0.8.7 staging branch, not the 0.8 this release branch.

@veggiemike
Copy link
Contributor Author

Shoot, I may have a problem. I just tried to update kernel modules on a virtual machine using a zfs root filesystem via rsync and got this:

rsync: write failed on "/vm/t/lib/modules/5.10.1-mdl+/modules.alias": File too large (27)
rsync error: error in file IO (code 11) at receiver.c(393) [receiver=3.1.2]

Looks like I'm getting a slew of "File to o large" errors reported in syslog, too.

With the system booted back into 5.9 w/ ZFS 0.8.6 I don't have any problems... so... either my backport isn't quite right, or I'm missing something else to get things fully working in 5.10.

@behlendorf
Copy link
Contributor

behlendorf commented Dec 20, 2020 •
edited
Loading

There are two follow commits you'll want to include #11373 #11378. Though from the error message it doesn't sound like you hit the #11378 issue. The CI tests also indicate this isn't quite right.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 20, 2020
@veggiemike
Copy link
Contributor Author

There are two follow commits you'll want to include #11373 #11378. Though from the error message it doesn't sound like you hit the #11378 issue. The CI tests also indicate this isn't quite right.

I cherry-picked those 2 commits and tried again. Same behavior. I've also cherry-picked all 3 onto ZFS 2.0 with the exact same behavior (but much simpler backport).

@behlendorf behlendorf mentioned this pull request Jan 11, 2021
Closed
13 tasks
@veggiemike
Copy link
Contributor Author

There are two follow commits you'll want to include #11373 #11378. Though from the error message it doesn't sound like you hit the #11378 issue. The CI tests also indicate this isn't quite right.

I cherry-picked those 2 commits and tried again. Same behavior. I've also cherry-picked all 3 onto ZFS 2.0 with the exact same behavior (but much simpler backport).

I did the same testing w/ 2.0.1-staging prior to its release and everything worked fine... So, whoever did the cherry-picking over the holidays did a better job than I did. ;-)

@behlendorf, I'll close this PR until I get around to comparing 2.0.1's contents to my branch... which might not happen, as I'm probably going to transition my systems to 2.0 at this point.

@veggiemike veggiemike closed this Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@veggiemike @PrivatePuffin @behlendorf @ckane