Linux 5.2 compat: Remove config/kernel-set-fs-pwd.m4 #8777

kusumi · 2019-05-21T12:20:22Z

Motivation and Context

Fix ./configure failure on 5.2-rc1.
Note that ZoL still doesn't compile on 5.2 due to several other kernel side changes.

Description

This failed on 5.2-rc1 with error: unknown message, for set_fs_pwd()
not being visible in both const and non-const tests. set_fs_pwd() has
never been exported with exception of some distro kernels.

set_fs_pwd() wasn't used in ZoL to begin with, and the test result was
used for a spl function vn_set_fs_pwd().

Confirmed on Fedora 29 with 5.2-rc1 (but not 5.1 with same gcc).

How Has This Been Tested?

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)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the ZFS on Linux code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
All new and existing tests passed.
All commit messages are properly formatted and contain Signed-off-by.

codecov · 2019-05-21T21:12:09Z

Codecov Report

Merging #8777 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #8777      +/-   ##
==========================================
+ Coverage   78.75%   78.77%   +0.02%     
==========================================
  Files         382      382              
  Lines      117809   117809              
==========================================
+ Hits        92776    92804      +28     
+ Misses      25033    25005      -28

Flag	Coverage Δ
#kernel	`79.33% <ø> (+0.01%)`	⬆️
#user	`67.48% <ø> (-0.02%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3e5907...be4f5cc. Read the comment docs.

behlendorf

This looks good and I'm happy to see it go. We might be able to finally go one step farther and remove the sole caller of this function in module/zfs/zfs_ioctl.c:_init(). Which would be nice.

This failed on 5.2-rc1 with "error: unknown" message, for set_fs_pwd() not being visible in both const and non-const tests. This is caused by torvalds/linux@83da1bed86. It's configurable, but we would want to be able to compile with default kbuild setting. set_fs_pwd() has never been exported with exception of some distro kernels, and set_fs_pwd() wasn't used in ZoL to begin with. The test result was used for a spl function vn_set_fs_pwd(). Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com>

kusumi · 2019-05-25T16:45:38Z

We might be able to finally go one step farther and remove the sole caller of this function in module/zfs/zfs_ioctl.c:_init(). Which would be nice.

Does this imply the rest of initialization code in module_init() doesn't (or might not, needs checking ?) require cwd set to / ?

If that's the case, I'll look into it, though probably better to merge this one alone first for 5.2.

behlendorf · 2019-05-25T20:37:26Z

That's right. It may no longer be necessary to set the modules cwd to /, this was solely done early on to make the behavior conform to the default illumos behavior.

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here don't depend on cwd of the process, except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non- NULL, which is "/etc/zfs/zpool.cache" given cwd is "/", so just unconditionally use the absolute path without "./" as a prefix. This is also what FreeBSD does. Also remove unused function spa_boot_init(). This is the only user of spa_config_load() (whose behavior no longer depends on cwd of caller) besides zfs_ioctl.c:_init(). Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com>

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Also remove unused function spa_boot_init(). This is the only user of spa_config_load() (whose behavior no longer depends on cwd of caller) besides zfs_ioctl.c:_init(). Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com>

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com>

@behlendorf

Per suggestion from @behlendorf in #8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes #8826

This failed on 5.2-rc1 with "error: unknown" message, for set_fs_pwd() not being visible in both const and non-const tests. This is caused by torvalds/linux@83da1bed86. It's configurable, but we would want to be able to compile with default kbuild setting. set_fs_pwd() has never been exported with exception of some distro kernels, and set_fs_pwd() wasn't used in ZoL to begin with. The test result was used for a spl function vn_set_fs_pwd(). Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: loli10K <ezomori.nozomu@gmail.com> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes #8777

This failed on 5.2-rc1 with "error: unknown" message, for set_fs_pwd() not being visible in both const and non-const tests. This is caused by torvalds/linux@83da1bed86. It's configurable, but we would want to be able to compile with default kbuild setting. set_fs_pwd() has never been exported with exception of some distro kernels, and set_fs_pwd() wasn't used in ZoL to begin with. The test result was used for a spl function vn_set_fs_pwd(). Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: loli10K <ezomori.nozomu@gmail.com> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes openzfs#8777

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes openzfs#8826

This failed on 5.2-rc1 with "error: unknown" message, for set_fs_pwd() not being visible in both const and non-const tests. This is caused by torvalds/linux@83da1bed86. It's configurable, but we would want to be able to compile with default kbuild setting. set_fs_pwd() has never been exported with exception of some distro kernels, and set_fs_pwd() wasn't used in ZoL to begin with. The test result was used for a spl function vn_set_fs_pwd(). Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: loli10K <ezomori.nozomu@gmail.com> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes openzfs#8777

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes openzfs#8826

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes openzfs#8826

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes openzfs#8826

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes openzfs#8826

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes openzfs#8826

@behlendorf

Per suggestion from @behlendorf in openzfs#8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes openzfs#8826

@behlendorf

Per suggestion from @behlendorf in #8777, remove vn_set_fs_pwd() and vn_set_pwd() which are only used in zfs_ioctl.c:_init() while loading zfs.ko. The rest of initialization functions being called here after cwd set to / don't depend on cwd of the process except for spa_config_load(). spa_config_load() uses a relative path ".//etc/zfs/zpool.cache" when `rootdir` is non-NULL, which is "/etc/zfs/zpool.cache" given cwd is /, so just unconditionally use the absolute path without "./", so that `vn_set_pwd("/")` as well as the entire functions can be removed. This is also what FreeBSD does. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Tony Hutter <hutter2@llnl.gov> Signed-off-by: Tomohiro Kusumi <kusumi.tomohiro@osnexus.com> Closes #8826

loli10K added the Status: Code Review Needed Ready for review and testing label May 21, 2019

kusumi mentioned this pull request May 23, 2019

building against current linux-5.2 kernel - configure failure because of modpost warning checks #8799

Closed

behlendorf approved these changes May 23, 2019

View reviewed changes

behlendorf requested a review from tonyhutter May 23, 2019 21:11

behlendorf mentioned this pull request May 23, 2019

spl-vnode.c compilation error with kernel 5.1.4 openzfs/spl#717

Closed

behlendorf added the Type: Building Indicates an issue related to building binaries label May 23, 2019

behlendorf requested a review from loli10K May 25, 2019 00:14

loli10K approved these changes May 25, 2019

View reviewed changes

kusumi force-pushed the setfs1 branch from 45c472a to 40cd4b1 Compare May 25, 2019 15:06

kusumi force-pushed the setfs1 branch from 40cd4b1 to be4f5cc Compare May 25, 2019 15:07

behlendorf merged commit 4bb17eb into openzfs:master May 25, 2019

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels May 25, 2019

behlendorf added this to In progress in 0.8-release May 25, 2019

kusumi mentioned this pull request May 27, 2019

Remove vn_set_fs_pwd()/vn_set_pwd() (no need to be at / during insmod) #8826

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linux 5.2 compat: Remove config/kernel-set-fs-pwd.m4 #8777

Linux 5.2 compat: Remove config/kernel-set-fs-pwd.m4 #8777

kusumi commented May 21, 2019

codecov bot commented May 21, 2019 •

edited

behlendorf left a comment

kusumi commented May 25, 2019

behlendorf commented May 25, 2019

Linux 5.2 compat: Remove config/kernel-set-fs-pwd.m4 #8777

Linux 5.2 compat: Remove config/kernel-set-fs-pwd.m4 #8777

Conversation

kusumi commented May 21, 2019

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

codecov bot commented May 21, 2019 • edited

Codecov Report

behlendorf left a comment

Choose a reason for hiding this comment

kusumi commented May 25, 2019

behlendorf commented May 25, 2019

codecov bot commented May 21, 2019 •

edited