Linux 5.12: struct user_namespace* in fs operations arguments #11712

ckane · 2021-03-09T02:14:27Z

Motivation and Context

This is another large set of changes required for Linux 5.12 kernel compatibility. Picking up the baton from PR #11699, kernel commit e65ce2a50cf6af216bea6fd80d771fcbb4c0aaa1 added some capability to the Linux kernel supporting id-mapped mounts, which modified a lot of handlers to now require a struct user_namespace* argument to be passed to them in order to be compatible with 5.12.

Description

This PR should include all of the struct user_namespace* argument changes throughout the kernel necessary to make this package compatible with kernel 5.12.
~~As of 2021-03-09, this is still a WORK IN PROGRESS PR.~~
Candidate for merge.

How Has This Been Tested?

Running on my daily use system, compiled against various 5.11 and 5.12 kernels

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.

ckane · 2021-03-15T02:51:35Z

@behlendorf - I have verified this code is now compiling on linux-mainline (5.12rc-something) as well as still compiling on Linux 5.11.4. What I am unsure of is what (if anything) we need to do additionally in the filesystem code to actually make use of these struct usernamespace* arguments, as I did this largely unfamiliar with the whole "id-mapping" kernel project. I've read some high-level documentation, and don't have any specific use case requirements of my own that need it. So.... NEEDS TESTING - maybe it works fine so long as the user is also not trying to use that new Linux id-mapping capability with a ZFS-mounted filesystem?

The inode_owner_or_capable() function was updated in the 5.12 kernel to support idmapped mounts. As part of this change the user namespace is now passed as the first argument. For non-idmapped mounts, which are currently not supported, the initial user namespace may be passed which will result in identical behavior as before. Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>

In Linux 5.12, the filesystem API was modified to add a "struct user_namespace *" parameter to a number of handlers. One of these is the "set" member of the "struct xatter_handler" type. A new argument of type "struct user_namespace*" was inserted, which breaks compatibilty with older module code, in commit e65ce2a50cf6af216bea6fd80d771fcbb4c0aaa1. This modification was made to a bunch of other filesystem operation handlers/structs as well. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Kernel 5.12 introduced a struct user_namespace* argument to a lot of filesystem code to support idmapped mounts. Exchanged the setattr_prepare() function for a zpl_setattr_prepare() macro that accepts the user_namespace from kcred->user_ns, and updated the refs in the source code appropriately. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Signed-off-by: Coleman Kane <ckane@colemankane.org>

The first arg to ZFS_LINUX_TEST_RESULT_SYMBOL is the base .ko filename, used earlier in the test definition, but the second argument is the name of the symbol we're looking for being exported. While the filename changed to define a new test result, the exported symbol name didn't, as the function has the same name on kernel 5.12 as it does for linux<5.12. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Kernel 5.12 added the struct user_namespace* arg to the getattr method in the struct inode_operations definition, to support idmapped io operations. This change modifies the configure tests, as well as implements the appropriate macro definition to handle the linux 5.12 API change, for the getattr handler. Signed-off-by: Coleman Kane <ckane@colemankane.org>

A bunch of the implementation functions for the getattr handler in different parts of the driver need to be updated to match the Linux 5.12 idmapped capability. This adds a new first argument to all of them that is the struct user_namespace* argument, if support for it is detected in the kernel during configure. Signed-off-by: Coleman Kane <ckane@colemankane.org>

The zfs_getattr_fast() function also needs to accept and use the user_namespace argument, introduced in kernel 5.12, and as well every place that calls generic_fillattr() needs to adopt the new API. This commit adds M4 code to config/ that detects the newer generic_fillattr() API, and conditionally compiles code elsewhere accordingly. Signed-off-by: Coleman Kane <ckane@colemankane.org>

This commit implements a few small fixes to bring the zpl_snapdir_mkdir() implementation to compatibility with 5.12 and its idmapped mounts addition. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Update to chase that zfs_getattr_fast() will need to have the correct version called where getattr() requires a struct user_namespace* argument. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Some of the line continuations were converted from 4-spaces to a tab in my earlier edits. This commit restores them to project style guidelines. Additionally, there were a few other style regressions that I went through and resolved. Signed-off-by: Coleman Kane <ckane@colemankane.org>

This commit adjusts zpl_mkdir() so that it will support the user_namespace argument introduced in kernel version 5.12. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Simplify a macro #define and also conditionally compile the zpl_setattr() handler depending upon if we need the struct user_namespace* arg in its type definition, or not. Signed-off-by: Coleman Kane <ckane@colemankane.org>

This commit updates both of these so that if configure determines that the kernel FS API requires struct user_namespace* as the first arg, then the respective zpl_mknod and zpl_symlink functions are compiled to match. Signed-off-by: Coleman Kane <ckane@colemankane.org>

When configure identifies that create needs to accept a struct user_namespace* as its first argument, make sure that we compile an implementation of the function that adheres to this API. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Implement configure code that determines if the struct user_namespace* is required as the first argument to rename() in struct inode_operations, and if so, update zpl_rename implementation to match. Signed-off-by: Coleman Kane <ckane@colemankane.org>

ckane · 2021-03-15T03:04:01Z

Additionally, I rebased this against openzfs/zfs master again

Some more additions that update the ZPL_XATTR_SET_WRAPPER() macro such that it will build interface functions that adopt the struct user_namespace* argument as the second arg, if that is the implementation required in the kernel (determined by configure). This was added to support idmapped FS mounts in Linux 5.12. Signed-off-by: Coleman Kane <ckane@colemankane.org>

ckane · 2021-03-16T15:07:27Z

This has been working for about a day on my daily system running 5.12rc2 with no regressions. However, there is a small change introduced in 5.12rc3 that replaces the use of the BIO_MAX_PAGES macro with a new function bio_max_segs() that handles the MIN logic internally which is commonly used around the kernel to bound allocations. I'll be submitting that change as a separate PR after I get a chance to test it as well in my dev branch.

ckane · 2021-03-16T15:07:51Z

That said, I suspect this particular PR is good to go @behlendorf

behlendorf

There's a good description of how the new idmapped mounts work in the kernel commit message, torvalds/linux@7d6beb7. Aside from all the interfaces which gained an argument not to much actually changed. In order to support idmapped mounts we'd need to set FS_ALLOW_IDMAP flag in fs_flags. If we don't then the initial user namespace will to be passed which is equivalent to how things work now.

It looks like it might actually not be to much trouble to support them aside from all interface changes made here. We could probably do something like add the passed user_ns to the CRED we're already setting up and passing down to the lower ZFS layers. This should make it available in most places we'd need it. According to the kernel commit message test cases were added to xfstests as well in order to verify it's working correctly.

Functionally this change looks right to me. It resolves the build issue and lays some of the needed groundwork to eventually support idmapped namespaces.

module/os/linux/zfs/zfs_vnops_os.c

Instead of conditionally compiling zfs_getattr_fast(), always accept the struct user_namespace* arg in that function, and wrap the kernel's getattr_fast() with a zpl_getattr_fast() wrapper that discards the first arg in Linux <5.12. Have the getattr handler implementations simply pass the kcred->user_ns value when called in earlier kernels that don't support the new interface. Signed-off-by: Coleman Kane <ckane@colemankane.org>

ckane · 2021-03-17T17:36:28Z

Also added the other 5.12 breakage fix as its own PR, independent of this, since it is on a different feature: #11765

In Linux 5.12, the filesystem API was modified to support ipmapped mounts by adding a "struct user_namespace *" parameter to a number functions and VFS handlers. This change adds the needed autoconf macros to detect the new interfaces and updates the code appropriately. This change does not add support for idmapped mounts, instead it preserves the existing behavior by passing the initial user namespace where needed. A subsequent commit will be required to add support for idmapped mounted. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Co-authored-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Coleman Kane <ckane@colemankane.org> Closes openzfs#11712

In Linux 5.12, the filesystem API was modified to support ipmapped mounts by adding a "struct user_namespace *" parameter to a number functions and VFS handlers. This change adds the needed autoconf macros to detect the new interfaces and updates the code appropriately. This change does not add support for idmapped mounts, instead it preserves the existing behavior by passing the initial user namespace where needed. A subsequent commit will be required to add support for idmapped mounted. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Co-authored-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Coleman Kane <ckane@colemankane.org> Closes #11712 (cherry picked from commit e2a8296) Signed-off-by: Jonathon Fernyhough <jonathon@m2x.dev>

In Linux 5.12, the filesystem API was modified to support ipmapped mounts by adding a "struct user_namespace *" parameter to a number functions and VFS handlers. This change adds the needed autoconf macros to detect the new interfaces and updates the code appropriately. This change does not add support for idmapped mounts, instead it preserves the existing behavior by passing the initial user namespace where needed. A subsequent commit will be required to add support for idmapped mounted. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Co-authored-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Coleman Kane <ckane@colemankane.org> Closes openzfs#11712

In Linux 5.12, the filesystem API was modified to support ipmapped mounts by adding a "struct user_namespace *" parameter to a number functions and VFS handlers. This change adds the needed autoconf macros to detect the new interfaces and updates the code appropriately. This change does not add support for idmapped mounts, instead it preserves the existing behavior by passing the initial user namespace where needed. A subsequent commit will be required to add support for idmapped mounted. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Co-authored-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Coleman Kane <ckane@colemankane.org> Closes #11712 (cherry picked from commit e2a8296) Signed-off-by: Jonathon Fernyhough <jonathon@m2x.dev>

brauner · 2021-08-12T14:15:08Z

There's a good description of how the new idmapped mounts work in the kernel commit message, torvalds/linux@7d6beb7. Aside from all the interfaces which gained an argument not to much actually changed. In order to support idmapped mounts we'd need to set FS_ALLOW_IDMAP flag in fs_flags. If we don't then the initial user namespace will to be passed which is equivalent to how things work now.

Please also see
https://github.com/brauner/linux/blob/fs.idmapped.docs/Documentation/filesystems/idmappings.rst
for a detailed explanation of idmappings in general and about idmapped mounts specifically see:

https://github.com/brauner/linux/blob/fs.idmapped.docs/Documentation/filesystems/idmappings.rst#idmappings-on-idmapped-mounts

ff. and please also note:

https://github.com/brauner/linux/blob/fs.idmapped.docs/Documentation/filesystems/idmappings.rst#shortcircuting

In Linux 5.12, the filesystem API was modified to support ipmapped mounts by adding a "struct user_namespace *" parameter to a number functions and VFS handlers. This change adds the needed autoconf macros to detect the new interfaces and updates the code appropriately. This change does not add support for idmapped mounts, instead it preserves the existing behavior by passing the initial user namespace where needed. A subsequent commit will be required to add support for idmapped mounted. Reviewed-by: Tony Hutter <hutter2@llnl.gov> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Co-authored-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Coleman Kane <ckane@colemankane.org> Closes openzfs#11712 (cherry picked from commit e2a8296) Signed-off-by: Jonathon Fernyhough <jonathon@m2x.dev>

ckane changed the title ~~Linux 5.12 compat~~ Linux 5.12: struct user_namespace* in fs operations arguments Mar 9, 2021

behlendorf added the Status: Work in Progress Not yet ready for general review label Mar 12, 2021

ckane force-pushed the linux-5.12-compat branch 2 times, most recently from 92b7e62 to 8be658a Compare March 14, 2021 20:27

ckane force-pushed the linux-5.12-compat branch 2 times, most recently from efe4774 to f97bd4b Compare March 15, 2021 02:54

behlendorf and others added 16 commits March 14, 2021 22:59

Add missing "no" in compatibility check for setattr_prepare

56aa180

Signed-off-by: Coleman Kane <ckane@colemankane.org>

mkdir() updates for user_namespace arg addition in 5.12

70f3b8b

This commit implements a few small fixes to bring the zpl_snapdir_mkdir() implementation to compatibility with 5.12 and its idmapped mounts addition. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Use correct gersion of zfs_getattr_fast() for 5.12 kernel

ff82ed1

Update to chase that zfs_getattr_fast() will need to have the correct version called where getattr() requires a struct user_namespace* argument. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Update zpl_mkdir() for 5.12 user_namespace arg

beba965

This commit adjusts zpl_mkdir() so that it will support the user_namespace argument introduced in kernel version 5.12. Signed-off-by: Coleman Kane <ckane@colemankane.org>

Updates for setattr() 5.12 user_namespace support

dfbe695

Simplify a macro #define and also conditionally compile the zpl_setattr() handler depending upon if we need the struct user_namespace* arg in its type definition, or not. Signed-off-by: Coleman Kane <ckane@colemankane.org>

ckane force-pushed the linux-5.12-compat branch from f97bd4b to 53841ab Compare March 15, 2021 03:02

ckane force-pushed the linux-5.12-compat branch from 078316d to 6e8512d Compare March 15, 2021 21:08

behlendorf added the Status: Code Review Needed Ready for review and testing label Mar 16, 2021

behlendorf reviewed Mar 16, 2021

View reviewed changes

module/os/linux/zfs/zfs_vnops_os.c Outdated Show resolved Hide resolved

module/os/linux/zfs/zfs_vnops_os.c Outdated Show resolved Hide resolved

behlendorf removed the Status: Work in Progress Not yet ready for general review label Mar 16, 2021

behlendorf requested a review from tonyhutter March 16, 2021 21:53

ckane force-pushed the linux-5.12-compat branch from 04d4c20 to 58e2ab2 Compare March 16, 2021 23:58

behlendorf approved these changes Mar 17, 2021

View reviewed changes

ckane mentioned this pull request Mar 17, 2021

Linux 5.12 update: bio_max_segs() replaces BIO_MAX_PAGES #11765

Merged

13 tasks

tonyhutter approved these changes Mar 17, 2021

View reviewed changes

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Mar 17, 2021

behlendorf merged commit e2a8296 into openzfs:master Mar 20, 2021

mikroskeem mentioned this pull request Dec 30, 2021

Support for FS_ALLOW_IDMAP #12923

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linux 5.12: struct user_namespace* in fs operations arguments #11712

Linux 5.12: struct user_namespace* in fs operations arguments #11712

ckane commented Mar 9, 2021 •

edited

Loading

ckane commented Mar 15, 2021

ckane commented Mar 15, 2021

ckane commented Mar 16, 2021

ckane commented Mar 16, 2021

behlendorf left a comment

ckane commented Mar 17, 2021

brauner commented Aug 12, 2021

Linux 5.12: struct user_namespace* in fs operations arguments #11712

Linux 5.12: struct user_namespace* in fs operations arguments #11712

Conversation

ckane commented Mar 9, 2021 • edited Loading

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

ckane commented Mar 15, 2021

ckane commented Mar 15, 2021

ckane commented Mar 16, 2021

ckane commented Mar 16, 2021

behlendorf left a comment

Choose a reason for hiding this comment

ckane commented Mar 17, 2021

brauner commented Aug 12, 2021

ckane commented Mar 9, 2021 •

edited

Loading