dm-vdo / kvdo

A kernel module which provide a pool of deduplicated and/or compressed block storage.
GNU General Public License v2.0
239 stars 45 forks source link

KVDO build error on Linux 6.8rc4: implicit declarations #81

Open thememika opened 7 months ago

thememika commented 7 months ago

Hello. kvdo can't be built for Linux 6.8rc1 and above, simply because these symbols used by kvdo no longer exist in kernel:

blkdev_put()
blkdev_get_by_dev()
blkdev_get_by_path()

The errors one encounters when trying to build kvdo for Linux 6.8rcX are of type:

/******/linux/modules/vdo/vdo/io-factory.c:67:11: warning: implicit 
declaration of function ‘blkdev_get_by_dev’; did you mean ‘blkdev_get_no_open’? [-Wimplicit-function-declaration]     67 |   *bdev = blkdev_get_by_dev(device, BLK_FMODE, NULL, &hops);
      |           ^~~~~~~~~~~~~~~~~

There are also warnings being treated as errors, related to missing function prototypes:

/**********/linux/modules/vdo/vdo/device-config.c:555:5:
error: no previous prototype for parse_optional_arguments’ [-Werror=missing-prototypes]                                             555 | int parse_optional_arguments(struct dm_arg_set *arg_set, 
                                     |     ^~~~~~~~~~~~~~~~~~~~~~~~

When is it planned to add support for 6.8 kernels?

thememika commented 7 months ago

As a temporary fix, I created a patch which you can apply to your 6.8-rcX kernel to make VDO compile successfully. It was tested on Linux kernel 6.8-rc4-rt4. The patch simply adds the required functions which were removed from the kernel (see post above), by taking them from the previous Linux version (6.7.5). The affected files are block/bdev.c and include/linux/blkdev.h. It assumes that no changes were made to these files. To apply the patch, use patch -p1 < /path/to/patch/file on your Linux kernel source tree.

After the kernel is compiled, you have to edit kvdo's source file: vdo/Makefile. You need to remove the string containing -Werror. Otherwise, warnings will be treated as errors, and missing prototype definitions will cause the build to stop. Then, you can compile kvdo successfully, seeing only missing prototype warnings, which will not affect the functionality.

The patch for the Linux kernel:

diff -crB a/block/bdev.c b/block/bdev.c
*** a/block/bdev.c  2024-02-27 23:56:53.700908178 +0200
--- b/block/bdev.c  2024-02-28 00:07:04.425641214 +0200
***************
*** 1116,1121 ****
--- 1116,1258 ----
    iput(old_inode);
  }

+ void blkdev_put(struct block_device *bdev, void *holder)
+ {
+   struct gendisk *disk = bdev->bd_disk;
+ 
+   /*
+    * Sync early if it looks like we're the last one.  If someone else
+    * opens the block device between now and the decrement of bd_openers
+    * then we did a sync that we didn't need to, but that's not the end
+    * of the world and we want to avoid long (could be several minute)
+    * syncs while holding the mutex.
+    */
+   if (atomic_read(&bdev->bd_openers) == 1)
+       sync_blockdev(bdev);
+ 
+   mutex_lock(&disk->open_mutex);
+   if (holder)
+       bd_end_claim(bdev, holder);
+ 
+   /*
+    * Trigger event checking and tell drivers to flush MEDIA_CHANGE
+    * event.  This is to ensure detection of media removal commanded
+    * from userland - e.g. eject(1).
+    */
+   disk_flush_events(disk, DISK_EVENT_MEDIA_CHANGE);
+ 
+   if (bdev_is_partition(bdev))
+       blkdev_put_part(bdev);
+   else
+       blkdev_put_whole(bdev);
+   mutex_unlock(&disk->open_mutex);
+ 
+   module_put(disk->fops->owner);
+   blkdev_put_no_open(bdev);
+ }
+ 
+ struct block_device *blkdev_get_by_dev(dev_t dev, blk_mode_t mode, void *holder,
+       const struct blk_holder_ops *hops)
+ {
+   bool unblock_events = true;
+   struct block_device *bdev;
+   struct gendisk *disk;
+   int ret;
+ 
+   ret = devcgroup_check_permission(DEVCG_DEV_BLOCK,
+           MAJOR(dev), MINOR(dev),
+           ((mode & BLK_OPEN_READ) ? DEVCG_ACC_READ : 0) |
+           ((mode & BLK_OPEN_WRITE) ? DEVCG_ACC_WRITE : 0));
+   if (ret)
+       return ERR_PTR(ret);
+ 
+   bdev = blkdev_get_no_open(dev);
+   if (!bdev)
+       return ERR_PTR(-ENXIO);
+   disk = bdev->bd_disk;
+ 
+   if (holder) {
+       mode |= BLK_OPEN_EXCL;
+       ret = bd_prepare_to_claim(bdev, holder, hops);
+       if (ret)
+           goto put_blkdev;
+   } else {
+       if (WARN_ON_ONCE(mode & BLK_OPEN_EXCL)) {
+           ret = -EIO;
+           goto put_blkdev;
+       }
+   }
+ 
+   disk_block_events(disk);
+ 
+   mutex_lock(&disk->open_mutex);
+   ret = -ENXIO;
+   if (!disk_live(disk))
+       goto abort_claiming;
+   if (!try_module_get(disk->fops->owner))
+       goto abort_claiming;
+   if (bdev_is_partition(bdev))
+       ret = blkdev_get_part(bdev, mode);
+   else
+       ret = blkdev_get_whole(bdev, mode);
+   if (ret)
+       goto put_module;
+   if (holder) {
+       bd_finish_claiming(bdev, holder, hops);
+ 
+       /*
+        * Block event polling for write claims if requested.  Any write
+        * holder makes the write_holder state stick until all are
+        * released.  This is good enough and tracking individual
+        * writeable reference is too fragile given the way @mode is
+        * used in blkdev_get/put().
+        */
+       if ((mode & BLK_OPEN_WRITE) && !bdev->bd_write_holder &&
+           (disk->event_flags & DISK_EVENT_FLAG_BLOCK_ON_EXCL_WRITE)) {
+           bdev->bd_write_holder = true;
+           unblock_events = false;
+       }
+   }
+   mutex_unlock(&disk->open_mutex);
+ 
+   if (unblock_events)
+       disk_unblock_events(disk);
+   return bdev;
+ put_module:
+   module_put(disk->fops->owner);
+ abort_claiming:
+   if (holder)
+       bd_abort_claiming(bdev, holder);
+   mutex_unlock(&disk->open_mutex);
+   disk_unblock_events(disk);
+ put_blkdev:
+   blkdev_put_no_open(bdev);
+   return ERR_PTR(ret);
+ }
+ 
+ struct block_device *blkdev_get_by_path(const char *path, blk_mode_t mode,
+       void *holder, const struct blk_holder_ops *hops)
+ {
+   struct block_device *bdev;
+   dev_t dev;
+   int error;
+ 
+   error = lookup_bdev(path, &dev);
+   if (error)
+       return ERR_PTR(error);
+ 
+   bdev = blkdev_get_by_dev(dev, mode, holder, hops);
+   if (!IS_ERR(bdev) && (mode & BLK_OPEN_WRITE) && bdev_read_only(bdev)) {
+       blkdev_put(bdev, holder);
+       return ERR_PTR(-EACCES);
+   }
+ 
+   return bdev;
+ };
+ EXPORT_SYMBOL_GPL(blkdev_put);
+ EXPORT_SYMBOL_GPL(blkdev_get_by_dev);
+ EXPORT_SYMBOL_GPL(blkdev_get_by_path);
+ 
  /*
   * Handle STATX_DIOALIGN for block devices.
   *
diff -crB a/include/linux/blkdev.h b/include/linux/blkdev.h
*** a/include/linux/blkdev.h    2024-02-27 23:59:40.005314071 +0200
--- b/include/linux/blkdev.h    2024-02-28 00:09:06.747038083 +0200
***************
*** 1492,1498 ****
  /* just for blk-cgroup, don't use elsewhere */
  struct block_device *blkdev_get_no_open(dev_t dev);
  void blkdev_put_no_open(struct block_device *bdev);
! 
  struct block_device *I_BDEV(struct inode *inode);

  #ifdef CONFIG_BLOCK
--- 1492,1502 ----
  /* just for blk-cgroup, don't use elsewhere */
  struct block_device *blkdev_get_no_open(dev_t dev);
  void blkdev_put_no_open(struct block_device *bdev);
! void blkdev_put(struct block_device *bdev, void *holder);
! struct block_device *blkdev_get_by_dev(dev_t dev, blk_mode_t mode, void *holder,
!       const struct blk_holder_ops *hops);
! struct block_device *blkdev_get_by_path(const char *path, blk_mode_t mode,
!       void *holder, const struct blk_holder_ops *hops);
  struct block_device *I_BDEV(struct inode *inode);

  #ifdef CONFIG_BLOCK

Note that I would like this issue to be closed only when the actual fix is made to kvdo, to allow it to compile against newest kernels without needing a modification to kernel.

lorelei-sakai commented 7 months ago

Hi,

Sorry to take so long to reply to this. We've been busily working to get vdo ready to be included in the main Linux kernel. It should be available starting in Linux 6.9. So the best answer might be to wait a few weeks and use the in-kernel version. You can even get a preview through the linux-next tree: drivers/md/dm-vdo

Because we are focused on the latest version, it's not clear when or even whether we will update the code in this repository. This blkdev interface started changing in the Linux 6.5 kernel and it has continued to change every few versions, so it's a bit of a moving target. The patch you posted works reasonably well as a stopgap.