Linux 6.7 compat #15681

robn · 2023-12-16T13:40:05Z

Motivation and Context

Upcoming Linux 6.7 release will change a couple of interfaces. Adapt or die!

Description

Adjusts for three changes:

i_atime and i_mtime in struct inode are no longer directly accessible, but must use accessor functions
s_shrink in struct super_block is now a pointer rather than an embedded struct
The "shrinker" (ie inode reclaim) API has changed, such that the kernel now allocates shrinkers dynamically rather than allowing a static shrinker to be registered.

How Has This Been Tested?

Compiled against 6.7-rc5, 6.7-rc6 and 5.10.170 and light sanity checks done (create pool, write some data, scrub it, echo 3 > /proc/sys/vm/drop_caches).

No test runs yet. Also have not attempted to compile or check on 3.0-3.11 (HAVE_SINGLE_SHRINKER_CALLBACK). (I'll run more tests soon enough; I just wanted to get this up before I disappear for the night).

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:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

philmmanjaro · 2023-12-17T06:49:30Z

This doesn't compile when applied on top of zfs-dkms 2.2.2 and errors out regarding current_time issues:

In file included from /build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/zfs/sys/zpl.h:36,
                 from /build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/zfs/sys/policy.h:36,
                 from /build/linux67-zfs/src/zfs/2.2.2/build/module/zfs/dmu_objset.c:62:
/build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/kernel/linux/vfs_compat.h:419:1: error: return type is an incomplete type
  419 | current_time(struct inode *ip)
      | ^~~~~~~~~~~~
/build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/kernel/linux/vfs_compat.h:419:1: error: conflicting types for 'current_time'; have 'void(struct inode *)'
In file included from ./include/linux/compat.h:17,
                 from ./arch/x86/include/asm/ia32.h:8,
                 from ./arch/x86/include/asm/elf.h:10,
                 from ./include/linux/elf.h:6,
                 from ./include/linux/module.h:19,
                 from /build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/spl/sys/cred.h:27,
                 from /build/linux67-zfs/src/zfs/2.2.2/build/module/zfs/dmu_objset.c:40:
./include/linux/fs.h:1516:19: note: previous declaration of 'current_time' with type 'struct timespec64(struct inode *)'
 1516 | struct timespec64 current_time(struct inode *inode);
      |                   ^~~~~~~~~~~~
/build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/kernel/linux/vfs_compat.h: In function 'current_time':
/build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/kernel/linux/vfs_compat.h:421:17: error: implicit declaration of function 'timespec_trunc'; did you mean 'timespec64_to_ns'? [-Werror=implicit-function-declaration]
  421 |         return (timespec_trunc(current_kernel_time(), ip->i_sb->s_time_gran));
      |                 ^~~~~~~~~~~~~~
      |                 timespec64_to_ns
/build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/kernel/linux/vfs_compat.h:421:32: error: implicit declaration of function 'current_kernel_time'; did you mean 'current_time'? [-Werror=implicit-function-declaration]
  421 |         return (timespec_trunc(current_kernel_time(), ip->i_sb->s_time_gran));
      |                                ^~~~~~~~~~~~~~~~~~~
      |                                current_time
/build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/kernel/linux/vfs_compat.h:421:17: error: 'return' with a value, in function returning void [-Werror=return-type]
  421 |         return (timespec_trunc(current_kernel_time(), ip->i_sb->s_time_gran));
      |                ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/build/linux67-zfs/src/zfs/2.2.2/build/include/os/linux/kernel/linux/vfs_compat.h:419:1: note: declared here
  419 | current_time(struct inode *ip)
      | ^~~~~~~~~~~~
cc1: some warnings being treated as errors

Build log: make.log

robn · 2023-12-17T10:17:21Z

@philmmanjaro here's linux-6.7.compat-2.2. This builds against 5.10.170, and 6.7-rc5, and a deb-dkms package built and installed fine against 6.1.52 on a Debian 12 system. So I'm gonna need more info: system you build the dkms package on, how you did it, and then system and kernel you're trying to build and install it on.

spikerguy · 2023-12-17T16:38:46Z

Hi @robn
I tried the 4 patches on zfs-dkms-git as well as zfs-dkms-2.2.2 both failed to compile.
Logs attached.

zfs-2.2.2-aarch64-6.7-rc3.txt

zfs-dkms-git--aarch64-6.7-rc3.txt

If you need any other detailed please feel free to ask.

Thanks.

robn · 2023-12-17T19:22:43Z

Ahh, sorta maybe seeing what's up. Can you post your build/build.log.kabi please? This is the output from the configure checks.

spikerguy · 2023-12-17T20:12:29Z

Hello
Sure.

Here:
zfs-2.2.2.-aarch64-6.7-rc3-kabi.log

robn · 2023-12-17T21:11:11Z

Thanks! The change in cdc0e7f is implicated, and though I don't entirely see why, I did think it felt brittle when I wrote it. I'll redo it a better way.

6.7 changed the names of the time members in struct inode, so we can't assign back to it because we don't know its name. In practice this doesn't matter though - if we're missing current_time(), then we must be on <4.9, and we know our fallback will need to return timespec. Signed-off-by: Rob Norris <robn@despairlabs.com> Sponsored-by: https://github.com/sponsors/robn

6.6 made i_ctime inaccessible; 6.7 has done the same for i_atime and i_mtime. This extends the method used for ctime in b37f293 to atime and mtime as well. Signed-off-by: Rob Norris <robn@despairlabs.com> Sponsored-by: https://github.com/sponsors/robn

In 6.7 the superblock shrinker member s_shrink has changed from being an embedded struct to a pointer. Detect this, and don't take a reference if it already is one. Signed-off-by: Rob Norris <robn@despairlabs.com> Sponsored-by: https://github.com/sponsors/robn

6.7 changes the shrinker API such that shrinkers must be allocated dynamically by the kernel. To accomodate this, this commit reworks spl_register_shrinker() to do something similar against earlier kernels. Signed-off-by: Rob Norris <robn@despairlabs.com> Sponsored-by: https://github.com/sponsors/robn

robn · 2023-12-17T22:52:02Z

I pushed a different way to do the current_time() check. See if that does better for you!

philmmanjaro · 2023-12-17T23:41:09Z

Works now also on my end: zfs-linux67.txt

spikerguy · 2023-12-18T08:40:50Z

Hey @robn
Thanks, It compiled fine on aarch64 using 6.7-rc3.

JohnyPeaN · 2023-12-18T14:39:56Z

Can confirm. Compiles and runs fine on 6.7-rc5.

6.6 made i_ctime inaccessible; 6.7 has done the same for i_atime and i_mtime. This extends the method used for ctime in b37f293 to atime and mtime as well. 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://github.com/sponsors/robn Closes #15681

In 6.7 the superblock shrinker member s_shrink has changed from being an embedded struct to a pointer. Detect this, and don't take a reference if it already is one. 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://github.com/sponsors/robn Closes #15681

6.7 changes the shrinker API such that shrinkers must be allocated dynamically by the kernel. To accomodate this, this commit reworks spl_register_shrinker() to do something similar against earlier kernels. 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://github.com/sponsors/robn Closes #15681

6.7 changed the names of the time members in struct inode, so we can't assign back to it because we don't know its name. In practice this doesn't matter though - if we're missing current_time(), then we must be on <4.9, and we know our fallback will need to return timespec. 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://github.com/sponsors/robn Closes openzfs#15681

6.6 made i_ctime inaccessible; 6.7 has done the same for i_atime and i_mtime. This extends the method used for ctime in b37f293 to atime and mtime as well. 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://github.com/sponsors/robn Closes openzfs#15681

In 6.7 the superblock shrinker member s_shrink has changed from being an embedded struct to a pointer. Detect this, and don't take a reference if it already is one. 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://github.com/sponsors/robn Closes openzfs#15681

6.7 changes the shrinker API such that shrinkers must be allocated dynamically by the kernel. To accomodate this, this commit reworks spl_register_shrinker() to do something similar against earlier kernels. 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://github.com/sponsors/robn Closes openzfs#15681