Linux 6.5 blkdev change fixes #15099

ckane · 2023-07-23T05:01:24Z

Motivation and Context

Multiple changes to the blkdev API were introduced in Linux 6.5. This includes passing (void* holder) to blkdev_put, adding a new blk_holder_ops* arg to blkdev_get_by_path, adding a new blk_mode_t type that replaces uses of fmode_t, and removing an argument from the release handler on block_device_operations that we weren't using.

Should fix: #15046

Description

Added new checks for:

4-arg blkdev_get_by_path()
blk_mode_t definition
blkdev_put() that takes void* as arg 2
bops->release() that takes 1 arg

Updates to code in module/os/linux/zfs/vdev_disk.c and module/os/linux/zfs/zvol_os.c to conditionally compile Linux 6.5 compatible code

How Has This Been Tested?

Tested no regression on 6.4.4 (uses old code). Compile tested on 6.5rc1, but cannot run-test yet due to other breaking changes.

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.

behlendorf

Thanks again for working through all these upcoming compatibility issues. A few comments inline.

module/os/linux/zfs/vdev_disk.c

module/os/linux/zfs/zvol_os.c

behlendorf · 2023-07-24T20:22:35Z

module/os/linux/zfs/zvol_os.c

@@ -769,14 +783,22 @@ zvol_open(struct block_device *bdev, fmode_t flag)
 			}
 		}

+#ifdef HAVE_BLK_MODE_T
+		error = -zvol_first_open(zv, !(flag & BLK_OPEN_WRITE));


Maybe add a blk_mode_is_open_write() wrapper, or some such function.

#ifdef HAVE_BLK_MODE_T #define blk_mode_is_open_write(flag) ((flag) & BLK_OPEN_WRITE) #else #define blk_mode_is_open_write(flag) ((flag) & FMODE_WRITE) #endif

behlendorf · 2023-07-24T20:26:52Z

module/os/linux/zfs/zvol_os.c

@@ -791,13 +813,21 @@ zvol_open(struct block_device *bdev, fmode_t flag)
 		rw_exit(&zv->zv_suspend_lock);

 	if (error == 0)
+#ifdef HAVE_BLK_MODE_T
+		disk_check_media_change(disk);
+#else
 		zfs_check_media_change(bdev);


Isn't this already handled by #15101?

module/os/linux/zfs/zvol_os.c

behlendorf · 2023-07-24T20:30:10Z

config/kernel-block-device-operations.m4

+dnl #
+dnl # 5.9.x API change
+dnl #
+AC_DEFUN([ZFS_AC_KERNEL_SRC_BLOCK_DEVICE_OPERATIONS_RELEASE_VOID_1ARG], [


Suggested change

AC_DEFUN([ZFS_AC_KERNEL_SRC_BLOCK_DEVICE_OPERATIONS_RELEASE_VOID_1ARG], [

AC_DEFUN([ZFS_AC_KERNEL_SRC_BLOCK_DEVICE_OPERATIONS_RELEASE_1ARG], [

I believe VOID here was is reference to the number of arguments and not the return type.

Multiple changes to the blkdev API were introduced in Linux 6.5. This includes passing (void* holder) to blkdev_put, adding a new blk_holder_ops* arg to blkdev_get_by_path, adding a new blk_mode_t type that replaces uses of fmode_t, and removing an argument from the release handler on block_device_operations that we weren't using. The open funtion definition has also changed to take gendisk* and blk_mode_t, so update it accordingly, too. Signed-off-by: Coleman Kane <[email protected]>

Implement local wrappers for blkdev_get_by_path() and vdev_blkdev_put() so that the in-line calls are cleaner, and place the conditionally-compiled implementation details inside of both of these local wrappers. Both calls are exclusively used within vdev_disk.c, at this time. Signed-off-by: Coleman Kane <[email protected]>

Signed-off-by: Coleman Kane <[email protected]>

The wrapper function is now used for testing using the appropriate method for the kernel, whether the open mode is writeable or not. Signed-off-by: Coleman Kane <[email protected]>

Signed-off-by: Coleman Kane <[email protected]>

behlendorf

Thanks, just a couple remaining nits.

module/os/linux/zfs/vdev_disk.c

module/os/linux/zfs/zvol_os.c

ckane · 2023-07-31T17:51:53Z

Ok all of those last 3 style suggestions should be addressed with the most recent commit

behlendorf · 2023-07-31T20:54:00Z

Well I stand corrected, there are some CentOS 7 build failures which the CI caught:

http://build.zfsonlinux.org/builders/CentOS%207%20x86_64%20%28TEST%29/builds/25084/steps/shell_1/logs/make

ckane · 2023-07-31T21:07:09Z

Well I stand corrected, there are some CentOS 7 build failures which the CI caught:

http://build.zfsonlinux.org/builders/CentOS%207%20x86_64%20%28TEST%29/builds/25084/steps/shell_1/logs/make

Ah ok, so I suppose I should revert the static elimination I did?

behlendorf · 2023-07-31T22:10:29Z

Right, we wanted to drop the inline not the static keyword.

ckane · 2023-08-01T00:39:11Z

Right, we wanted to drop the inline not the static keyword.

Oops I realize just now I misread the original suggestion

Signed-off-by: Coleman Kane <[email protected]>

ckane · 2023-08-01T12:25:33Z

Alright, looks like all checks passed this time around

Multiple changes to the blkdev API were introduced in Linux 6.5. This includes passing (void* holder) to blkdev_put, adding a new blk_holder_ops* arg to blkdev_get_by_path, adding a new blk_mode_t type that replaces uses of fmode_t, and removing an argument from the release handler on block_device_operations that we weren't using. The open function definition has also changed to take gendisk* and blk_mode_t, so update it accordingly, too. Implement local wrappers for blkdev_get_by_path() and vdev_blkdev_put() so that the in-line calls are cleaner, and place the conditionally-compiled implementation details inside of both of these local wrappers. Both calls are exclusively used within vdev_disk.c, at this time. Add blk_mode_is_open_write() to test FMODE_WRITE / BLK_OPEN_WRITE The wrapper function is now used for testing using the appropriate method for the kernel, whether the open mode is writable or not. Emphasize fmode_t arg in zvol_release is not used Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Coleman Kane <[email protected]> Closes openzfs#15099

Multiple changes to the blkdev API were introduced in Linux 6.5. This includes passing (void* holder) to blkdev_put, adding a new blk_holder_ops* arg to blkdev_get_by_path, adding a new blk_mode_t type that replaces uses of fmode_t, and removing an argument from the release handler on block_device_operations that we weren't using. The open function definition has also changed to take gendisk* and blk_mode_t, so update it accordingly, too. Implement local wrappers for blkdev_get_by_path() and vdev_blkdev_put() so that the in-line calls are cleaner, and place the conditionally-compiled implementation details inside of both of these local wrappers. Both calls are exclusively used within vdev_disk.c, at this time. Add blk_mode_is_open_write() to test FMODE_WRITE / BLK_OPEN_WRITE The wrapper function is now used for testing using the appropriate method for the kernel, whether the open mode is writable or not. Emphasize fmode_t arg in zvol_release is not used Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Coleman Kane <[email protected]> Closes #15099

Multiple changes to the blkdev API were introduced in Linux 6.5. This includes passing (void* holder) to blkdev_put, adding a new blk_holder_ops* arg to blkdev_get_by_path, adding a new blk_mode_t type that replaces uses of fmode_t, and removing an argument from the release handler on block_device_operations that we weren't using. The open function definition has also changed to take gendisk* and blk_mode_t, so update it accordingly, too. Implement local wrappers for blkdev_get_by_path() and vdev_blkdev_put() so that the in-line calls are cleaner, and place the conditionally-compiled implementation details inside of both of these local wrappers. Both calls are exclusively used within vdev_disk.c, at this time. Add blk_mode_is_open_write() to test FMODE_WRITE / BLK_OPEN_WRITE The wrapper function is now used for testing using the appropriate method for the kernel, whether the open mode is writable or not. Emphasize fmode_t arg in zvol_release is not used Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Coleman Kane <[email protected]> Closes openzfs#15099

asdil12 · 2023-12-19T13:31:47Z

There are cases where building fails, because even though we are in the HAVE_BLKDEV_GET_BY_PATH_4ARG case, BLK_OPEN_EXCL is undefined and FMODE_EXCL needs to be used:

https://github.com/openzfs/zfs/pull/15099/files#diff-e8ebc1c455c1b37f292f3a9a03dc5aab357073bbeb4a77540e881ee271b311a3R226-R232

Workaround:

diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c
index 48ac55f07..a81c5eaff 100644
--- a/module/os/linux/zfs/vdev_disk.c
+++ b/module/os/linux/zfs/vdev_disk.c
@@ -224,6 +224,9 @@ vdev_blkdev_get_by_path(const char *path, spa_mode_t mode, void *holder,
     const struct blk_holder_ops *hops)
 {
 #ifdef HAVE_BLKDEV_GET_BY_PATH_4ARG
+#if !defined(BLK_OPEN_EXCL)
+#define BLK_OPEN_EXCL FMODE_EXCL
+#endif
 	return (blkdev_get_by_path(path,
 	    vdev_bdev_mode(mode) | BLK_OPEN_EXCL, holder, hops));
 #else

behlendorf · 2023-12-19T16:45:50Z

@asdil12 can you open a PR with the proposed fix. I agree we should handle this since we can.

asdil12 · 2023-12-19T19:06:36Z

#15688

ckane force-pushed the linux-6.5-blkdev-changes branch 2 times, most recently from 7c447ae to 2c7b4c8 Compare July 23, 2023 06:08

behlendorf reviewed Jul 24, 2023

View reviewed changes

behlendorf added the Status: Code Review Needed Ready for review and testing label Jul 24, 2023

ckane added 2 commits July 30, 2023 13:45

ckane force-pushed the linux-6.5-blkdev-changes branch from 2c7b4c8 to a9b203b Compare July 30, 2023 18:25

ckane added 3 commits July 30, 2023 15:33

Emphasize fmode_t arg in zvol_release is not used

9e920ec

Signed-off-by: Coleman Kane <[email protected]>

Shorten macro & configure test names for 1-arg release handler test

40e11da

Signed-off-by: Coleman Kane <[email protected]>

Add blk_mode_is_open_write() to test FMODE_WRITE / BLK_OPEN_WRITE

5379a40

The wrapper function is now used for testing using the appropriate method for the kernel, whether the open mode is writeable or not. Signed-off-by: Coleman Kane <[email protected]>

ckane force-pushed the linux-6.5-blkdev-changes branch from a11927e to 5379a40 Compare July 30, 2023 19:56

Fix typo in HAVE_BLKDEV_GET_BY_PATH_4ARG

de56924

Signed-off-by: Coleman Kane <[email protected]>

behlendorf reviewed Jul 31, 2023

View reviewed changes

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

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

module/os/linux/zfs/zvol_os.c Show resolved Hide resolved

ckane force-pushed the linux-6.5-blkdev-changes branch from b8cdd0d to a2cab05 Compare July 31, 2023 17:50

Fix a complaint from cstyle and un-inline two helper functions

db407f5

Signed-off-by: Coleman Kane <[email protected]>

ckane force-pushed the linux-6.5-blkdev-changes branch from a2cab05 to db407f5 Compare August 1, 2023 00:39

behlendorf approved these changes Aug 1, 2023

View reviewed changes

behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Aug 1, 2023

behlendorf merged commit 43e8f6e into openzfs:master Aug 1, 2023

satmandu mentioned this pull request Aug 2, 2023

Linux 6.5 compat: UBSAN now complains about flex-array declarations with array[1] #15145

Closed

asdil12 mentioned this pull request Dec 19, 2023

Ensure BLK_OPEN_EXCL is defined in HAVE_BLKDEV_GET_BY_PATH_4ARG case #15688

Closed

behlendorf mentioned this pull request Dec 20, 2023

Linux 6.5 compat: check BLK_OPEN_EXCL is defined #15692

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linux 6.5 blkdev change fixes #15099

Linux 6.5 blkdev change fixes #15099

ckane commented Jul 23, 2023

behlendorf left a comment

behlendorf Jul 24, 2023

behlendorf Jul 24, 2023

behlendorf Jul 24, 2023

behlendorf left a comment

ckane commented Jul 31, 2023

behlendorf commented Jul 31, 2023

ckane commented Jul 31, 2023

behlendorf commented Jul 31, 2023

ckane commented Aug 1, 2023

ckane commented Aug 1, 2023

asdil12 commented Dec 19, 2023

behlendorf commented Dec 19, 2023

asdil12 commented Dec 19, 2023

	AC_DEFUN([ZFS_AC_KERNEL_SRC_BLOCK_DEVICE_OPERATIONS_RELEASE_VOID_1ARG], [
	AC_DEFUN([ZFS_AC_KERNEL_SRC_BLOCK_DEVICE_OPERATIONS_RELEASE_1ARG], [

Linux 6.5 blkdev change fixes #15099

Linux 6.5 blkdev change fixes #15099

Conversation

ckane commented Jul 23, 2023

Motivation and Context

Description

How Has This Been Tested?

Types of changes

Checklist:

behlendorf left a comment

Choose a reason for hiding this comment

behlendorf Jul 24, 2023

Choose a reason for hiding this comment

behlendorf Jul 24, 2023

Choose a reason for hiding this comment

behlendorf Jul 24, 2023

Choose a reason for hiding this comment

behlendorf left a comment

Choose a reason for hiding this comment

ckane commented Jul 31, 2023

behlendorf commented Jul 31, 2023

ckane commented Jul 31, 2023

behlendorf commented Jul 31, 2023

ckane commented Aug 1, 2023

ckane commented Aug 1, 2023

asdil12 commented Dec 19, 2023

behlendorf commented Dec 19, 2023

asdil12 commented Dec 19, 2023