From: Michael Jeanson Date: Mon, 10 May 2021 15:39:24 +0000 (-0400) Subject: fix: block: remove disk_part_iter (v5.12) X-Git-Url: https://git.lttng.org/?p=lttng-modules.git;a=commitdiff_plain;h=481fd2a270a95e667695199d6a3d376cda374540 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 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 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 Signed-off-by: Mathieu Desnoyers --- diff --git a/include/wrapper/genhd.h b/include/wrapper/genhd.h index c28a52e3..68980388 100644 --- a/include/wrapper/genhd.h +++ b/include/wrapper/genhd.h @@ -13,13 +13,6 @@ #define _LTTNG_WRAPPER_GENHD_H #include -#include - -#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 0eba20d8..7137874f 100644 --- 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 394f6618..a9a757bc 100644 --- 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 index a5a6c410..00000000 --- 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 - */ - -#include -#include -#include - -#if (defined(CONFIG_KALLSYMS) && \ - (LTTNG_LINUX_VERSION_CODE >= LTTNG_KERNEL_VERSION(5,12,0))) - -#include - -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