fix: block: remove disk_part_iter (v5.12)

In v5.12 a refactoring of the genhd code was started and the symbols
related to 'disk_part_iter' were unexported. In v5.13 they were
completely removed.

This patch replaces the short lived compat code that is specific to
v5.12 and replaces it with a generic internal implementation that
iterates directly on the 'disk->part_tbl' xarray which will be used
on v5.12 and up.

This seems like a better option than keeping the compat code that will
only work on v5.12 and make maintenance more complicated. The compat was
backported to the stable branches but isn't yet part of a point release
so can be safely replaced.

See the upstream commits:

  commit 3212135a718b06be38811f2d9a320ae842e76409
  Author: Christoph Hellwig <hch@lst.de>
  Date:   Tue Apr 6 08:23:02 2021 +0200

    block: remove disk_part_iter

    Just open code the xa_for_each in the remaining user.

  commit a33df75c6328bf40078b35f2040d8e54d574c357
  Author: Christoph Hellwig <hch@lst.de>
  Date:   Sun Jan 24 11:02:41 2021 +0100

    block: use an xarray for disk->part_tbl

    Now that no fast path lookups in the partition table are left, there is
    no point in micro-optimizing the data structure for it.  Just use a bog
    standard xarray.

Change-Id: If3497e087fadaa285e135f57aab7e6df157b06c6
Signed-off-by: Michael Jeanson <mjeanson@efficios.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/include/wrapper/genhd.h b/include/wrapper/genhd.h
index c28a52e343dfe730d1a98ecc2395080c04cb1910..689803881dbb0f9a950c651d48aa4b9a11e83564 100644 (file)
--- a/include/wrapper/genhd.h
+++ b/include/wrapper/genhd.h
@@ -13,13 +13,6 @@
 #define _LTTNG_WRAPPER_GENHD_H
 
 #include <linux/genhd.h>
-#include <lttng/kernel-version.h>
-
-#if (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,11,0))
-#define LTTNG_DISK_PART_TYPE struct block_device
-#else
-#define LTTNG_DISK_PART_TYPE struct hd_struct
-#endif
 
 #ifdef CONFIG_KALLSYMS_ALL
 
@@ -101,59 +94,4 @@ struct device_type *wrapper_get_disk_type(void)
 
 #endif
 
-/*
- * This wrapper has an 'int' return type instead of the original 'void', to be
- * able to report the symbol lookup failure to the caller.
- *
- * Return 0 on success, -1 on error.
- */
-int wrapper_disk_part_iter_init(struct disk_part_iter *piter, struct gendisk *disk,
-                          unsigned int flags);
-LTTNG_DISK_PART_TYPE *wrapper_disk_part_iter_next(struct disk_part_iter *piter);
-void wrapper_disk_part_iter_exit(struct disk_part_iter *piter);
-
-/*
- * Canary function to check for 'disk_part_iter_init()' at compile time.
- *
- * From 'include/linux/genhd.h':
- *
- *   extern void disk_part_iter_init(struct disk_part_iter *piter,
- *                                   struct gendisk *disk, unsigned int flags);
- *
- */
-static inline
-void __canary__disk_part_iter_init(struct disk_part_iter *piter, struct gendisk *disk,
-               unsigned int flags)
-{
-       disk_part_iter_init(piter, disk, flags);
-}
-
-/*
- * Canary function to check for 'disk_part_iter_next()' at compile time.
- *
- * From 'include/linux/genhd.h':
- *
- *   struct block_device *disk_part_iter_next(struct disk_part_iter *piter);
- *
- */
-static inline
-LTTNG_DISK_PART_TYPE *__canary__disk_part_iter_next(struct disk_part_iter *piter)
-{
-       return disk_part_iter_next(piter);
-}
-
-/*
- * Canary function to check for 'disk_part_iter_exit()' at compile time.
- *
- * From 'include/linux/genhd.h':
- *
- *   extern void disk_part_iter_exit(struct disk_part_iter *piter);
- *
- */
-static inline
-void __canary__disk_part_iter_exit(struct disk_part_iter *piter)
-{
-       return disk_part_iter_exit(piter);
-}
-
 #endif /* _LTTNG_WRAPPER_GENHD_H */

diff --git a/src/Kbuild b/src/Kbuild
index 0eba20d81b3db8f0fecf33a2ff024e5155048627..7137874fa4c7a0bf0a7ce899ff97a2c340f91aa8 100644 (file)
--- a/src/Kbuild
+++ b/src/Kbuild
@@ -85,7 +85,6 @@ lttng-wrapper-objs := wrapper/page_alloc.o \
                       wrapper/kallsyms.o \
                       wrapper/irqdesc.o \
                       wrapper/fdtable.o \
-                      wrapper/genhd.o \
                       lttng-wrapper-impl.o
 
 ifneq ($(CONFIG_HAVE_SYSCALL_TRACEPOINTS),)

diff --git a/src/lttng-statedump-impl.c b/src/lttng-statedump-impl.c
index 394f661861a88a8f00a52002b2440112b9df2b68..a9a757bcd1415748b778cf8c31b7b85eaed51183 100644 (file)
--- a/src/lttng-statedump-impl.c
+++ b/src/lttng-statedump-impl.c
@@ -249,6 +249,61 @@ dev_t lttng_get_part_devt(struct hd_struct *part)
 }
 #endif
 
+#if (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,12,0))
+static
+int lttng_statedump_each_block_device(struct lttng_kernel_session *session, struct gendisk *disk)
+{
+       struct block_device *part;
+       unsigned long idx;
+       int ret = 0;
+
+       /* Include partition 0 */
+       idx = 0;
+
+       rcu_read_lock();
+       xa_for_each(&disk->part_tbl, idx, part) {
+               char name_buf[BDEVNAME_SIZE];
+
+               /* Exclude non-partitions bdev and empty partitions. */
+               if (bdev_is_partition(part) && !bdev_nr_sectors(part))
+                       continue;
+
+               if (lttng_get_part_name(disk, part, name_buf) == -ENOSYS) {
+                       ret = -ENOSYS;
+                       goto end;
+               }
+               trace_lttng_statedump_block_device(session, lttng_get_part_devt(part),
+                               name_buf);
+       }
+end:
+       rcu_read_unlock();
+       return ret;
+}
+#else
+static
+int lttng_statedump_each_block_device(struct lttng_kernel_session *session, struct gendisk *disk)
+{
+       struct disk_part_iter piter;
+       LTTNG_PART_STRUCT_TYPE *part;
+
+       disk_part_iter_init(&piter, disk, DISK_PITER_INCL_PART0);
+
+       while ((part = disk_part_iter_next(&piter))) {
+               char name_buf[BDEVNAME_SIZE];
+
+               if (lttng_get_part_name(disk, part, name_buf) == -ENOSYS) {
+                       disk_part_iter_exit(&piter);
+                       return -ENOSYS;
+               }
+               trace_lttng_statedump_block_device(session, lttng_get_part_devt(part),
+                               name_buf);
+       }
+       disk_part_iter_exit(&piter);
+
+       return 0;
+}
+#endif
+
 static
 int lttng_enumerate_block_devices(struct lttng_kernel_session *session)
 {
@@ -270,9 +325,7 @@ int lttng_enumerate_block_devices(struct lttng_kernel_session *session)
        }
        class_dev_iter_init(&iter, ptr_block_class, NULL, ptr_disk_type);
        while ((dev = class_dev_iter_next(&iter))) {
-               struct disk_part_iter piter;
                struct gendisk *disk = dev_to_disk(dev);
-               LTTNG_PART_STRUCT_TYPE *part;
 
                /*
                 * Don't show empty devices or things that have been
@@ -282,29 +335,8 @@ int lttng_enumerate_block_devices(struct lttng_kernel_session *session)
                    (disk->flags & GENHD_FL_SUPPRESS_PARTITION_INFO))
                        continue;
 
-               /*
-                * The original 'disk_part_iter_init' returns void, but our
-                * wrapper can fail to lookup the original symbol.
-                */
-               if (wrapper_disk_part_iter_init(&piter, disk, DISK_PITER_INCL_PART0) < 0) {
-                       ret = -ENOSYS;
-                       goto iter_exit;
-               }
-
-               while ((part = wrapper_disk_part_iter_next(&piter))) {
-                       char name_buf[BDEVNAME_SIZE];
-
-                       if (lttng_get_part_name(disk, part, name_buf) == -ENOSYS) {
-                               wrapper_disk_part_iter_exit(&piter);
-                               ret = -ENOSYS;
-                               goto iter_exit;
-                       }
-                       trace_lttng_statedump_block_device(session,
-                                       lttng_get_part_devt(part), name_buf);
-               }
-               wrapper_disk_part_iter_exit(&piter);
+               ret = lttng_statedump_each_block_device(session, disk);
        }
-iter_exit:
        class_dev_iter_exit(&iter);
 end:
        return ret;

diff --git a/src/wrapper/genhd.c b/src/wrapper/genhd.c
deleted file mode 100644 (file)
index a5a6c41..0000000
--- a/src/wrapper/genhd.c
+++ /dev/null
@@ -1,111 +0,0 @@
-/* SPDX-License-Identifier: (GPL-2.0-only OR LGPL-2.1-only)
- *
- * wrapper/genhd.c
- *
- * Wrapper around disk_part_iter_(init|next|exit). Using KALLSYMS to get the
- * addresses when available, else we need to have a kernel that exports this
- * function to GPL modules. This export was removed in 5.12.
- *
- * Copyright (C) 2021 Michael Jeanson <mjeanson@efficios.com>
- */
-
-#include <lttng/kernel-version.h>
-#include <linux/module.h>
-#include <wrapper/genhd.h>
-
-#if (defined(CONFIG_KALLSYMS) && \
-       (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,12,0)))
-
-#include <wrapper/kallsyms.h>
-
-static
-void (*disk_part_iter_init_sym)(struct disk_part_iter *piter, struct gendisk *disk,
-                       unsigned int flags);
-
-static
-LTTNG_DISK_PART_TYPE *(*disk_part_iter_next_sym)(struct disk_part_iter *piter);
-
-static
-void (*disk_part_iter_exit_sym)(struct disk_part_iter *piter);
-
-/*
- * This wrapper has an 'int' return type instead of the original 'void', to be
- * able to report the symbol lookup failure to the caller.
- *
- * Return 0 on success, -1 on error.
- */
-int wrapper_disk_part_iter_init(struct disk_part_iter *piter, struct gendisk *disk,
-                          unsigned int flags)
-{
-       if (!disk_part_iter_init_sym)
-               disk_part_iter_init_sym = (void *) kallsyms_lookup_funcptr("disk_part_iter_init");
-
-       if (disk_part_iter_init_sym) {
-               disk_part_iter_init_sym(piter, disk, flags);
-       } else {
-               printk_once(KERN_WARNING "LTTng: disk_part_iter_init symbol lookup failed.\n");
-               return -1;
-       }
-       return 0;
-}
-EXPORT_SYMBOL_GPL(wrapper_disk_part_iter_init);
-
-LTTNG_DISK_PART_TYPE *wrapper_disk_part_iter_next(struct disk_part_iter *piter)
-{
-       if (!disk_part_iter_next_sym)
-               disk_part_iter_next_sym = (void *) kallsyms_lookup_funcptr("disk_part_iter_next");
-
-       if (disk_part_iter_next_sym) {
-               return disk_part_iter_next_sym(piter);
-       } else {
-               printk_once(KERN_WARNING "LTTng: disk_part_iter_next symbol lookup failed.\n");
-               return NULL;
-       }
-}
-EXPORT_SYMBOL_GPL(wrapper_disk_part_iter_next);
-
-/*
- * We don't return an error on symbol lookup failure here because there is
- * nothing the caller can do to cleanup the iterator.
- */
-void wrapper_disk_part_iter_exit(struct disk_part_iter *piter)
-{
-       if (!disk_part_iter_exit_sym)
-               disk_part_iter_exit_sym = (void *) kallsyms_lookup_funcptr("disk_part_iter_exit");
-
-       if (disk_part_iter_exit_sym) {
-               disk_part_iter_exit_sym(piter);
-       } else {
-               printk_once(KERN_WARNING "LTTng: disk_part_iter_exit symbol lookup failed.\n");
-       }
-}
-EXPORT_SYMBOL_GPL(wrapper_disk_part_iter_exit);
-
-#else
-
-/*
- * This wrapper has an 'int' return type instead of the original 'void', so the
- * kallsyms variant can report the symbol lookup failure to the caller.
- *
- * This variant always succeeds and returns 0.
- */
-int wrapper_disk_part_iter_init(struct disk_part_iter *piter, struct gendisk *disk,
-                          unsigned int flags)
-{
-       disk_part_iter_init(piter, disk, flags);
-       return 0;
-}
-EXPORT_SYMBOL_GPL(wrapper_disk_part_iter_init);
-
-LTTNG_DISK_PART_TYPE *wrapper_disk_part_iter_next(struct disk_part_iter *piter)
-{
-       return disk_part_iter_next(piter);
-}
-EXPORT_SYMBOL_GPL(wrapper_disk_part_iter_next);
-
-void wrapper_disk_part_iter_exit(struct disk_part_iter *piter)
-{
-       disk_part_iter_exit(piter);
-}
-EXPORT_SYMBOL_GPL(wrapper_disk_part_iter_exit);
-#endif