From 6efea26f09f23ea8f3235158bd1d7bf5c6278d4f Mon Sep 17 00:00:00 2001 From: Rob N Date: Sat, 30 Mar 2024 08:51:33 +1100 Subject: [PATCH 1/4] vdev_disk: clean up spa/bdev mode conversion 43e8f6e37 introduced a subtle API misuse, in that it passed the output from vdev_bdev_mode() back into itself. Fortunately, the SPA_MODE_(READ|WRITE) bit values exactly map to the FMODE_(READ|WRITE) & BLK_OPEN_(READ|WRITE) bit values, so it didn't result in a bug, but it was hard to read and understand, so I cleaned it up. In doing so, I noticed that the only call to vdev_bdev_mode() without the "exclusive" flag set was in that misuse, and actually, we never do a non-exclusive blkdev_get_by_path(). So I've just made exclusive be always-on. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Reviewed by: Brian Behlendorf Reviewed-by: Allan Jude Signed-off-by: Rob Norris Closes #15995 --- module/os/linux/zfs/vdev_disk.c | 81 ++++++++++++++++----------------- 1 file changed, 39 insertions(+), 42 deletions(-) diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c index 3ec4f0f09696..737df0f79b52 100644 --- a/module/os/linux/zfs/vdev_disk.c +++ b/module/os/linux/zfs/vdev_disk.c @@ -94,38 +94,41 @@ typedef struct dio_request { struct bio *dr_bio[0]; /* Attached bio's */ } dio_request_t; +/* + * Convert SPA mode flags into bdev open mode flags. + */ #ifdef HAVE_BLK_MODE_T -static blk_mode_t +typedef blk_mode_t vdev_bdev_mode_t; +#define VDEV_BDEV_MODE_READ BLK_OPEN_READ +#define VDEV_BDEV_MODE_WRITE BLK_OPEN_WRITE +#define VDEV_BDEV_MODE_EXCL BLK_OPEN_EXCL +#define VDEV_BDEV_MODE_MASK (BLK_OPEN_READ|BLK_OPEN_WRITE|BLK_OPEN_EXCL) #else -static fmode_t +typedef fmode_t vdev_bdev_mode_t; +#define VDEV_BDEV_MODE_READ FMODE_READ +#define VDEV_BDEV_MODE_WRITE FMODE_WRITE +#define VDEV_BDEV_MODE_EXCL FMODE_EXCL +#define VDEV_BDEV_MODE_MASK (FMODE_READ|FMODE_WRITE|FMODE_EXCL) #endif -vdev_bdev_mode(spa_mode_t spa_mode, boolean_t exclusive) -{ -#ifdef HAVE_BLK_MODE_T - blk_mode_t mode = 0; - - if (spa_mode & SPA_MODE_READ) - mode |= BLK_OPEN_READ; - if (spa_mode & SPA_MODE_WRITE) - mode |= BLK_OPEN_WRITE; +static vdev_bdev_mode_t +vdev_bdev_mode(spa_mode_t smode) +{ + ASSERT3U(smode, !=, SPA_MODE_UNINIT); + ASSERT0(smode & ~(SPA_MODE_READ|SPA_MODE_WRITE)); - if (exclusive) - mode |= BLK_OPEN_EXCL; -#else - fmode_t mode = 0; + vdev_bdev_mode_t bmode = VDEV_BDEV_MODE_EXCL; - if (spa_mode & SPA_MODE_READ) - mode |= FMODE_READ; + if (smode & SPA_MODE_READ) + bmode |= VDEV_BDEV_MODE_READ; - if (spa_mode & SPA_MODE_WRITE) - mode |= FMODE_WRITE; + if (smode & SPA_MODE_WRITE) + bmode |= VDEV_BDEV_MODE_WRITE; - if (exclusive) - mode |= FMODE_EXCL; -#endif + ASSERT(bmode & VDEV_BDEV_MODE_MASK); + ASSERT0(bmode & ~VDEV_BDEV_MODE_MASK); - return (mode); + return (bmode); } /* @@ -232,30 +235,28 @@ vdev_disk_kobj_evt_post(vdev_t *v) } static zfs_bdev_handle_t * -vdev_blkdev_get_by_path(const char *path, spa_mode_t mode, void *holder) +vdev_blkdev_get_by_path(const char *path, spa_mode_t smode, void *holder) { + vdev_bdev_mode_t bmode = vdev_bdev_mode(smode); + #if defined(HAVE_BDEV_OPEN_BY_PATH) - return (bdev_open_by_path(path, - vdev_bdev_mode(mode, B_TRUE), holder, NULL)); + return (bdev_open_by_path(path, bmode, holder, NULL)); #elif defined(HAVE_BLKDEV_GET_BY_PATH_4ARG) - return (blkdev_get_by_path(path, - vdev_bdev_mode(mode, B_TRUE), holder, NULL)); + return (blkdev_get_by_path(path, bmode, holder, NULL)); #else - return (blkdev_get_by_path(path, - vdev_bdev_mode(mode, B_TRUE), holder)); + return (blkdev_get_by_path(path, bmode, holder)); #endif } static void -vdev_blkdev_put(zfs_bdev_handle_t *bdh, spa_mode_t mode, void *holder) +vdev_blkdev_put(zfs_bdev_handle_t *bdh, spa_mode_t smode, void *holder) { #if defined(HAVE_BDEV_RELEASE) return (bdev_release(bdh)); #elif defined(HAVE_BLKDEV_PUT_HOLDER) return (blkdev_put(BDH_BDEV(bdh), holder)); #else - return (blkdev_put(BDH_BDEV(bdh), - vdev_bdev_mode(mode, B_TRUE))); + return (blkdev_put(BDH_BDEV(bdh), vdev_bdev_mode(smode))); #endif } @@ -264,11 +265,7 @@ vdev_disk_open(vdev_t *v, uint64_t *psize, uint64_t *max_psize, uint64_t *logical_ashift, uint64_t *physical_ashift) { zfs_bdev_handle_t *bdh; -#ifdef HAVE_BLK_MODE_T - blk_mode_t mode = vdev_bdev_mode(spa_mode(v->vdev_spa), B_FALSE); -#else - fmode_t mode = vdev_bdev_mode(spa_mode(v->vdev_spa), B_FALSE); -#endif + spa_mode_t smode = spa_mode(v->vdev_spa); hrtime_t timeout = MSEC2NSEC(zfs_vdev_open_timeout_ms); vdev_disk_t *vd; @@ -319,16 +316,16 @@ vdev_disk_open(vdev_t *v, uint64_t *psize, uint64_t *max_psize, reread_part = B_TRUE; } - vdev_blkdev_put(bdh, mode, zfs_vdev_holder); + vdev_blkdev_put(bdh, smode, zfs_vdev_holder); } if (reread_part) { - bdh = vdev_blkdev_get_by_path(disk_name, mode, + bdh = vdev_blkdev_get_by_path(disk_name, smode, zfs_vdev_holder); if (!BDH_IS_ERR(bdh)) { int error = vdev_bdev_reread_part(BDH_BDEV(bdh)); - vdev_blkdev_put(bdh, mode, zfs_vdev_holder); + vdev_blkdev_put(bdh, smode, zfs_vdev_holder); if (error == 0) { timeout = MSEC2NSEC( zfs_vdev_open_timeout_ms * 2); @@ -373,7 +370,7 @@ vdev_disk_open(vdev_t *v, uint64_t *psize, uint64_t *max_psize, hrtime_t start = gethrtime(); bdh = BDH_ERR_PTR(-ENXIO); while (BDH_IS_ERR(bdh) && ((gethrtime() - start) < timeout)) { - bdh = vdev_blkdev_get_by_path(v->vdev_path, mode, + bdh = vdev_blkdev_get_by_path(v->vdev_path, smode, zfs_vdev_holder); if (unlikely(BDH_PTR_ERR(bdh) == -ENOENT)) { /* From aabb7e63ba9fc04a36c7412cae4649ffde1e7915 Mon Sep 17 00:00:00 2001 From: Rob Norris Date: Wed, 27 Mar 2024 10:07:50 +1100 Subject: [PATCH 2/4] Linux 6.9 compat: bdev handles are now struct file bdev_open_by_path() is replaced by bdev_file_open_by_path(), which returns a plain old struct file*. Release function is gone entirely; the regular file release function fput() will take care of the bdev specifics. Reviewed-by: Tony Hutter Reviewed-by: Brian Behlendorf Signed-off-by: Rob Norris Sponsored-by: https://despairlabs.com/sponsor/ Closes #16027 Closes #16033 --- config/kernel-blkdev.m4 | 43 +++++++++++++++++++++++++++++++-- module/os/linux/zfs/vdev_disk.c | 24 ++++++++++++++---- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/config/kernel-blkdev.m4 b/config/kernel-blkdev.m4 index 8e9e638b125a..f390b3e65968 100644 --- a/config/kernel-blkdev.m4 +++ b/config/kernel-blkdev.m4 @@ -54,6 +54,26 @@ AC_DEFUN([ZFS_AC_KERNEL_SRC_BLKDEV_BDEV_OPEN_BY_PATH], [ ]) ]) +dnl # +dnl # 6.9.x API change +dnl # bdev_file_open_by_path() replaced bdev_open_by_path(), +dnl # and returns struct file* +dnl # +AC_DEFUN([ZFS_AC_KERNEL_SRC_BDEV_FILE_OPEN_BY_PATH], [ + ZFS_LINUX_TEST_SRC([bdev_file_open_by_path], [ + #include + #include + ], [ + struct file *file __attribute__ ((unused)) = NULL; + const char *path = "path"; + fmode_t mode = 0; + void *holder = NULL; + struct blk_holder_ops h; + + file = bdev_file_open_by_path(path, mode, holder, &h); + ]) +]) + AC_DEFUN([ZFS_AC_KERNEL_BLKDEV_GET_BY_PATH], [ AC_MSG_CHECKING([whether blkdev_get_by_path() exists and takes 3 args]) ZFS_LINUX_TEST_RESULT([blkdev_get_by_path], [ @@ -73,7 +93,16 @@ AC_DEFUN([ZFS_AC_KERNEL_BLKDEV_GET_BY_PATH], [ [bdev_open_by_path() exists]) AC_MSG_RESULT(yes) ], [ - ZFS_LINUX_TEST_ERROR([blkdev_get_by_path()]) + AC_MSG_RESULT(no) + AC_MSG_CHECKING([whether bdev_file_open_by_path() exists]) + ZFS_LINUX_TEST_RESULT([bdev_file_open_by_path], [ + AC_DEFINE(HAVE_BDEV_FILE_OPEN_BY_PATH, 1, + [bdev_file_open_by_path() exists]) + AC_MSG_RESULT(yes) + ], [ + AC_MSG_RESULT(no) + ZFS_LINUX_TEST_ERROR([blkdev_get_by_path()]) + ]) ]) ]) ]) @@ -149,10 +178,19 @@ AC_DEFUN([ZFS_AC_KERNEL_SRC_BLKDEV_BDEV_RELEASE], [ ]) ]) +dnl # +dnl # 6.9.x API change +dnl # +dnl # bdev_release() now private, but because bdev_file_open_by_path() returns +dnl # struct file*, we can just use fput(). So the blkdev_put test no longer +dnl # fails if not found. +dnl # + AC_DEFUN([ZFS_AC_KERNEL_BLKDEV_PUT], [ AC_MSG_CHECKING([whether blkdev_put() exists]) ZFS_LINUX_TEST_RESULT([blkdev_put], [ AC_MSG_RESULT(yes) + AC_DEFINE(HAVE_BLKDEV_PUT, 1, [blkdev_put() exists]) ], [ AC_MSG_RESULT(no) AC_MSG_CHECKING([whether blkdev_put() accepts void* as arg 2]) @@ -168,7 +206,7 @@ AC_DEFUN([ZFS_AC_KERNEL_BLKDEV_PUT], [ AC_DEFINE(HAVE_BDEV_RELEASE, 1, [bdev_release() exists]) ], [ - ZFS_LINUX_TEST_ERROR([blkdev_put()]) + AC_MSG_RESULT(no) ]) ]) ]) @@ -621,6 +659,7 @@ AC_DEFUN([ZFS_AC_KERNEL_SRC_BLKDEV], [ ZFS_AC_KERNEL_SRC_BLKDEV_GET_BY_PATH ZFS_AC_KERNEL_SRC_BLKDEV_GET_BY_PATH_4ARG ZFS_AC_KERNEL_SRC_BLKDEV_BDEV_OPEN_BY_PATH + ZFS_AC_KERNEL_SRC_BDEV_FILE_OPEN_BY_PATH ZFS_AC_KERNEL_SRC_BLKDEV_PUT ZFS_AC_KERNEL_SRC_BLKDEV_PUT_HOLDER ZFS_AC_KERNEL_SRC_BLKDEV_BDEV_RELEASE diff --git a/module/os/linux/zfs/vdev_disk.c b/module/os/linux/zfs/vdev_disk.c index 737df0f79b52..2cd7fe22c48d 100644 --- a/module/os/linux/zfs/vdev_disk.c +++ b/module/os/linux/zfs/vdev_disk.c @@ -44,15 +44,25 @@ /* * Linux 6.8.x uses a bdev_handle as an instance/refcount for an underlying * block_device. Since it carries the block_device inside, its convenient to - * just use the handle as a proxy. For pre-6.8, we just emulate this with - * a cast, since we don't need any of the other fields inside the handle. + * just use the handle as a proxy. + * + * Linux 6.9.x uses a file for the same purpose. + * + * For pre-6.8, we just emulate this with a cast, since we don't need any of + * the other fields inside the handle. */ -#ifdef HAVE_BDEV_OPEN_BY_PATH +#if defined(HAVE_BDEV_OPEN_BY_PATH) typedef struct bdev_handle zfs_bdev_handle_t; #define BDH_BDEV(bdh) ((bdh)->bdev) #define BDH_IS_ERR(bdh) (IS_ERR(bdh)) #define BDH_PTR_ERR(bdh) (PTR_ERR(bdh)) #define BDH_ERR_PTR(err) (ERR_PTR(err)) +#elif defined(HAVE_BDEV_FILE_OPEN_BY_PATH) +typedef struct file zfs_bdev_handle_t; +#define BDH_BDEV(bdh) (file_bdev(bdh)) +#define BDH_IS_ERR(bdh) (IS_ERR(bdh)) +#define BDH_PTR_ERR(bdh) (PTR_ERR(bdh)) +#define BDH_ERR_PTR(err) (ERR_PTR(err)) #else typedef void zfs_bdev_handle_t; #define BDH_BDEV(bdh) ((struct block_device *)bdh) @@ -239,7 +249,9 @@ vdev_blkdev_get_by_path(const char *path, spa_mode_t smode, void *holder) { vdev_bdev_mode_t bmode = vdev_bdev_mode(smode); -#if defined(HAVE_BDEV_OPEN_BY_PATH) +#if defined(HAVE_BDEV_FILE_OPEN_BY_PATH) + return (bdev_file_open_by_path(path, bmode, holder, NULL)); +#elif defined(HAVE_BDEV_OPEN_BY_PATH) return (bdev_open_by_path(path, bmode, holder, NULL)); #elif defined(HAVE_BLKDEV_GET_BY_PATH_4ARG) return (blkdev_get_by_path(path, bmode, holder, NULL)); @@ -255,8 +267,10 @@ vdev_blkdev_put(zfs_bdev_handle_t *bdh, spa_mode_t smode, void *holder) return (bdev_release(bdh)); #elif defined(HAVE_BLKDEV_PUT_HOLDER) return (blkdev_put(BDH_BDEV(bdh), holder)); -#else +#elif defined(HAVE_BLKDEV_PUT) return (blkdev_put(BDH_BDEV(bdh), vdev_bdev_mode(smode))); +#else + fput(bdh); #endif } From 621a06b1904679eb3031dca8fa17d4cba88f9171 Mon Sep 17 00:00:00 2001 From: Tony Hutter Date: Mon, 2 Dec 2024 15:50:11 -0800 Subject: [PATCH 3/4] [zfs-2.1.16] Workaround for Ubuntu runners When Alien converts the RPMs to DEB packages, it calls this line: $this->do("rpm2cpio '".$this->filename."' | (cd $workdir; $decomp cpio --extract --make-directories --no-absolute-filenames --preserve-modification-time) 2>&1") This seems to fail on zfs-2.1.16 with: cpio: ./usr/src/spl-2.1.16/6.5.0-1025-azure: Cannot open: No such file or directory The workaround is just to always return true from the 'cpio' command. This does not seem to affect anything. Signed-off-by: Tony Hutter --- .github/workflows/zfs-tests-functional.yml | 16 ++++++++++++++++ .github/workflows/zfs-tests-sanity.yml | 16 ++++++++++++++++ .github/workflows/zloop.yml | 16 ++++++++++++++++ 3 files changed, 48 insertions(+) diff --git a/.github/workflows/zfs-tests-functional.yml b/.github/workflows/zfs-tests-functional.yml index 1d30ddc645a4..8d3ed3151254 100644 --- a/.github/workflows/zfs-tests-functional.yml +++ b/.github/workflows/zfs-tests-functional.yml @@ -31,6 +31,22 @@ jobs: ./configure --enable-debug --enable-debuginfo - name: Make run: | + # Hack for zfs-2.1.16+ + # + # When Alien converts the RPMs to DEB packages, it calls this line: + # + # $this->do("rpm2cpio '".$this->filename."' | (cd $workdir; $decomp cpio + # --extract --make-directories --no-absolute-filenames + # --preserve-modification-time) 2>&1") + # + # This seems to fail with zfs-2.1.16 with: + # + # cpio: ./usr/src/spl-2.1.16/6.5.0-1025-azure: Cannot open: No such file or directory + # + # The workaround is just to always return true from the 'cpio' command. + # This does not seem to affect anything. + sudo sed -i 's/preserve-modification-time/preserve-modification-time | true/g' /usr/share/perl5/Alien/Package/Rpm.pm + make --no-print-directory -s pkg-utils pkg-kmod - name: Install run: | diff --git a/.github/workflows/zfs-tests-sanity.yml b/.github/workflows/zfs-tests-sanity.yml index 6a1432c2972f..84f4b7f6aaa4 100644 --- a/.github/workflows/zfs-tests-sanity.yml +++ b/.github/workflows/zfs-tests-sanity.yml @@ -27,6 +27,22 @@ jobs: ./configure --enable-debug --enable-debuginfo - name: Make run: | + # Hack for zfs-2.1.16+ + # + # When Alien converts the RPMs to DEB packages, it calls this line: + # + # $this->do("rpm2cpio '".$this->filename."' | (cd $workdir; $decomp cpio + # --extract --make-directories --no-absolute-filenames + # --preserve-modification-time) 2>&1") + # + # This seems to fail with zfs-2.1.16 with: + # + # cpio: ./usr/src/spl-2.1.16/6.5.0-1025-azure: Cannot open: No such file or directory + # + # The workaround is just to always return true from the 'cpio' command. + # This does not seem to affect anything. + sudo sed -i 's/preserve-modification-time/preserve-modification-time | true/g' /usr/share/perl5/Alien/Package/Rpm.pm + make --no-print-directory -s pkg-utils pkg-kmod - name: Install run: | diff --git a/.github/workflows/zloop.yml b/.github/workflows/zloop.yml index 440ec01faa9f..018c55c4bc5c 100644 --- a/.github/workflows/zloop.yml +++ b/.github/workflows/zloop.yml @@ -29,6 +29,22 @@ jobs: ./configure --enable-debug --enable-debuginfo - name: Make run: | + # Hack for zfs-2.1.16+ + # + # When Alien converts the RPMs to DEB packages, it calls this line: + # + # $this->do("rpm2cpio '".$this->filename."' | (cd $workdir; $decomp cpio + # --extract --make-directories --no-absolute-filenames + # --preserve-modification-time) 2>&1") + # + # This seems to fail with zfs-2.1.16 with: + # + # cpio: ./usr/src/spl-2.1.16/6.5.0-1025-azure: Cannot open: No such file or directory + # + # The workaround is just to always return true from the 'cpio' command. + # This does not seem to affect anything. + sudo sed -i 's/preserve-modification-time/preserve-modification-time | true/g' /usr/share/perl5/Alien/Package/Rpm.pm + make --no-print-directory -s pkg-utils pkg-kmod - name: Install run: | From a7186651d3306debca6b4f72797239eea61db36c Mon Sep 17 00:00:00 2001 From: Tony Hutter Date: Mon, 18 Nov 2024 13:17:58 -0800 Subject: [PATCH 4/4] Tag zfs-2.1.16 META file and changelog updated. Signed-off-by: Tony Hutter --- META | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/META b/META index 92d538495cc3..5b4888e7e672 100644 --- a/META +++ b/META @@ -1,7 +1,7 @@ Meta: 1 Name: zfs Branch: 1.0 -Version: 2.1.15 +Version: 2.1.16 Release: 1 Release-Tags: relext License: CDDL