From 371f88d96fe0aeb46a72fec78f90e1d777493ee5 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Thu, 10 Jun 2021 11:27:33 -0400 Subject: [PATCH] Remove pool io kstats (#12212) This mostly reverts "3537 want pool io kstats" commit of 8 years ago. From one side this code using pool-wide locks became pretty bad for performance, creating significant lock contention in I/O pipeline. From another, there are more efficient ways now to obtain detailed statistics, while this statistics is illumos-specific and much less usable on Linux and FreeBSD, reported only via procfs/sysctls. This commit does not remove KSTAT_TYPE_IO implementation, that may be removed later together with already unused KSTAT_TYPE_INTR and KSTAT_TYPE_TIMER. Reviewed-by: Matthew Ahrens Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored-By: iXsystems, Inc. Closes #12212 --- include/os/freebsd/spl/sys/kstat.h | 4 -- include/os/linux/spl/sys/kstat.h | 4 -- include/sys/spa.h | 1 - include/sys/zfs_context.h | 6 --- lib/libspl/include/sys/kstat.h | 6 --- lib/libzpool/kernel.c | 30 -------------- module/os/freebsd/spl/spl_kstat.c | 62 ---------------------------- module/os/linux/spl/spl-kstat.c | 66 ------------------------------ module/zfs/spa_stats.c | 50 ---------------------- module/zfs/vdev_queue.c | 47 --------------------- 10 files changed, 276 deletions(-) diff --git a/include/os/freebsd/spl/sys/kstat.h b/include/os/freebsd/spl/sys/kstat.h index f5157c7f4f..947dfee623 100644 --- a/include/os/freebsd/spl/sys/kstat.h +++ b/include/os/freebsd/spl/sys/kstat.h @@ -210,10 +210,6 @@ extern kstat_t *__kstat_create(const char *ks_module, int ks_instance, extern void __kstat_install(kstat_t *ksp); extern void __kstat_delete(kstat_t *ksp); -extern void kstat_waitq_enter(kstat_io_t *); -extern void kstat_waitq_exit(kstat_io_t *); -extern void kstat_runq_enter(kstat_io_t *); -extern void kstat_runq_exit(kstat_io_t *); #define kstat_set_seq_raw_ops(k, h, d, a) \ __kstat_set_seq_raw_ops(k, h, d, a) diff --git a/include/os/linux/spl/sys/kstat.h b/include/os/linux/spl/sys/kstat.h index 905d8257c8..928f707575 100644 --- a/include/os/linux/spl/sys/kstat.h +++ b/include/os/linux/spl/sys/kstat.h @@ -206,10 +206,6 @@ extern void kstat_proc_entry_install(kstat_proc_entry_t *kpep, mode_t mode, extern void __kstat_install(kstat_t *ksp); extern void __kstat_delete(kstat_t *ksp); -extern void kstat_waitq_enter(kstat_io_t *); -extern void kstat_waitq_exit(kstat_io_t *); -extern void kstat_runq_enter(kstat_io_t *); -extern void kstat_runq_exit(kstat_io_t *); #define kstat_set_raw_ops(k, h, d, a) \ __kstat_set_raw_ops(k, h, d, a) diff --git a/include/sys/spa.h b/include/sys/spa.h index 374d36e732..d37c6c923d 100644 --- a/include/sys/spa.h +++ b/include/sys/spa.h @@ -895,7 +895,6 @@ typedef struct spa_stats { spa_history_list_t read_history; spa_history_list_t txg_history; spa_history_kstat_t tx_assign_histogram; - spa_history_kstat_t io_history; spa_history_list_t mmp_history; spa_history_kstat_t state; /* pool state */ spa_history_kstat_t iostats; diff --git a/include/sys/zfs_context.h b/include/sys/zfs_context.h index 89afa98253..aa4338ed28 100644 --- a/include/sys/zfs_context.h +++ b/include/sys/zfs_context.h @@ -360,12 +360,6 @@ extern kstat_t *kstat_create(const char *, int, const char *, const char *, uchar_t, ulong_t, uchar_t); extern void kstat_install(kstat_t *); extern void kstat_delete(kstat_t *); -extern void kstat_waitq_enter(kstat_io_t *); -extern void kstat_waitq_exit(kstat_io_t *); -extern void kstat_runq_enter(kstat_io_t *); -extern void kstat_runq_exit(kstat_io_t *); -extern void kstat_waitq_to_runq(kstat_io_t *); -extern void kstat_runq_back_to_waitq(kstat_io_t *); extern void kstat_set_raw_ops(kstat_t *ksp, int (*headers)(char *buf, size_t size), int (*data)(char *buf, size_t size, void *data), diff --git a/lib/libspl/include/sys/kstat.h b/lib/libspl/include/sys/kstat.h index 69fb6d401f..f73fb92eb7 100644 --- a/lib/libspl/include/sys/kstat.h +++ b/lib/libspl/include/sys/kstat.h @@ -796,12 +796,6 @@ extern void kstat_delete_byname(const char *, int, const char *); extern void kstat_delete_byname_zone(const char *, int, const char *, zoneid_t); extern void kstat_named_init(kstat_named_t *, const char *, uchar_t); extern void kstat_timer_init(kstat_timer_t *, const char *); -extern void kstat_waitq_enter(kstat_io_t *); -extern void kstat_waitq_exit(kstat_io_t *); -extern void kstat_runq_enter(kstat_io_t *); -extern void kstat_runq_exit(kstat_io_t *); -extern void kstat_waitq_to_runq(kstat_io_t *); -extern void kstat_runq_back_to_waitq(kstat_io_t *); extern void kstat_timer_start(kstat_timer_t *); extern void kstat_timer_stop(kstat_timer_t *); diff --git a/lib/libzpool/kernel.c b/lib/libzpool/kernel.c index e96a1d7521..cc8e534e7e 100644 --- a/lib/libzpool/kernel.c +++ b/lib/libzpool/kernel.c @@ -146,36 +146,6 @@ void kstat_delete(kstat_t *ksp) {} -/*ARGSUSED*/ -void -kstat_waitq_enter(kstat_io_t *kiop) -{} - -/*ARGSUSED*/ -void -kstat_waitq_exit(kstat_io_t *kiop) -{} - -/*ARGSUSED*/ -void -kstat_runq_enter(kstat_io_t *kiop) -{} - -/*ARGSUSED*/ -void -kstat_runq_exit(kstat_io_t *kiop) -{} - -/*ARGSUSED*/ -void -kstat_waitq_to_runq(kstat_io_t *kiop) -{} - -/*ARGSUSED*/ -void -kstat_runq_back_to_waitq(kstat_io_t *kiop) -{} - void kstat_set_raw_ops(kstat_t *ksp, int (*headers)(char *buf, size_t size), diff --git a/module/os/freebsd/spl/spl_kstat.c b/module/os/freebsd/spl/spl_kstat.c index e591921ace..059ada235c 100644 --- a/module/os/freebsd/spl/spl_kstat.c +++ b/module/os/freebsd/spl/spl_kstat.c @@ -508,65 +508,3 @@ kstat_delete(kstat_t *ksp) kmem_free(ksp->ks_data, ksp->ks_data_size); free(ksp, M_KSTAT); } - -void -kstat_waitq_enter(kstat_io_t *kiop) -{ - hrtime_t new, delta; - ulong_t wcnt; - - new = gethrtime(); - delta = new - kiop->wlastupdate; - kiop->wlastupdate = new; - wcnt = kiop->wcnt++; - if (wcnt != 0) { - kiop->wlentime += delta * wcnt; - kiop->wtime += delta; - } -} - -void -kstat_waitq_exit(kstat_io_t *kiop) -{ - hrtime_t new, delta; - ulong_t wcnt; - - new = gethrtime(); - delta = new - kiop->wlastupdate; - kiop->wlastupdate = new; - wcnt = kiop->wcnt--; - ASSERT3S(wcnt, >, 0); - kiop->wlentime += delta * wcnt; - kiop->wtime += delta; -} - -void -kstat_runq_enter(kstat_io_t *kiop) -{ - hrtime_t new, delta; - ulong_t rcnt; - - new = gethrtime(); - delta = new - kiop->rlastupdate; - kiop->rlastupdate = new; - rcnt = kiop->rcnt++; - if (rcnt != 0) { - kiop->rlentime += delta * rcnt; - kiop->rtime += delta; - } -} - -void -kstat_runq_exit(kstat_io_t *kiop) -{ - hrtime_t new, delta; - ulong_t rcnt; - - new = gethrtime(); - delta = new - kiop->rlastupdate; - kiop->rlastupdate = new; - rcnt = kiop->rcnt--; - ASSERT3S(rcnt, >, 0); - kiop->rlentime += delta * rcnt; - kiop->rtime += delta; -} diff --git a/module/os/linux/spl/spl-kstat.c b/module/os/linux/spl/spl-kstat.c index c7f1aadf78..0c46708326 100644 --- a/module/os/linux/spl/spl-kstat.c +++ b/module/os/linux/spl/spl-kstat.c @@ -50,72 +50,6 @@ kstat_resize_raw(kstat_t *ksp) return (0); } -void -kstat_waitq_enter(kstat_io_t *kiop) -{ - hrtime_t new, delta; - ulong_t wcnt; - - new = gethrtime(); - delta = new - kiop->wlastupdate; - kiop->wlastupdate = new; - wcnt = kiop->wcnt++; - if (wcnt != 0) { - kiop->wlentime += delta * wcnt; - kiop->wtime += delta; - } -} -EXPORT_SYMBOL(kstat_waitq_enter); - -void -kstat_waitq_exit(kstat_io_t *kiop) -{ - hrtime_t new, delta; - ulong_t wcnt; - - new = gethrtime(); - delta = new - kiop->wlastupdate; - kiop->wlastupdate = new; - wcnt = kiop->wcnt--; - ASSERT((int)wcnt > 0); - kiop->wlentime += delta * wcnt; - kiop->wtime += delta; -} -EXPORT_SYMBOL(kstat_waitq_exit); - -void -kstat_runq_enter(kstat_io_t *kiop) -{ - hrtime_t new, delta; - ulong_t rcnt; - - new = gethrtime(); - delta = new - kiop->rlastupdate; - kiop->rlastupdate = new; - rcnt = kiop->rcnt++; - if (rcnt != 0) { - kiop->rlentime += delta * rcnt; - kiop->rtime += delta; - } -} -EXPORT_SYMBOL(kstat_runq_enter); - -void -kstat_runq_exit(kstat_io_t *kiop) -{ - hrtime_t new, delta; - ulong_t rcnt; - - new = gethrtime(); - delta = new - kiop->rlastupdate; - kiop->rlastupdate = new; - rcnt = kiop->rcnt--; - ASSERT((int)rcnt > 0); - kiop->rlentime += delta * rcnt; - kiop->rtime += delta; -} -EXPORT_SYMBOL(kstat_runq_exit); - static int kstat_seq_show_headers(struct seq_file *f) { diff --git a/module/zfs/spa_stats.c b/module/zfs/spa_stats.c index c3eacc1423..534ac72fee 100644 --- a/module/zfs/spa_stats.c +++ b/module/zfs/spa_stats.c @@ -548,54 +548,6 @@ spa_tx_assign_add_nsecs(spa_t *spa, uint64_t nsecs) atomic_inc_64(&((kstat_named_t *)shk->priv)[idx].value.ui64); } -/* - * ========================================================================== - * SPA IO History Routines - * ========================================================================== - */ -static int -spa_io_history_update(kstat_t *ksp, int rw) -{ - if (rw == KSTAT_WRITE) - memset(ksp->ks_data, 0, ksp->ks_data_size); - - return (0); -} - -static void -spa_io_history_init(spa_t *spa) -{ - spa_history_kstat_t *shk = &spa->spa_stats.io_history; - char *name; - kstat_t *ksp; - - mutex_init(&shk->lock, NULL, MUTEX_DEFAULT, NULL); - - name = kmem_asprintf("zfs/%s", spa_name(spa)); - - ksp = kstat_create(name, 0, "io", "disk", KSTAT_TYPE_IO, 1, 0); - shk->kstat = ksp; - - if (ksp) { - ksp->ks_lock = &shk->lock; - ksp->ks_private = spa; - ksp->ks_update = spa_io_history_update; - kstat_install(ksp); - } - kmem_strfree(name); -} - -static void -spa_io_history_destroy(spa_t *spa) -{ - spa_history_kstat_t *shk = &spa->spa_stats.io_history; - - if (shk->kstat) - kstat_delete(shk->kstat); - - mutex_destroy(&shk->lock); -} - /* * ========================================================================== * SPA MMP History Routines @@ -996,7 +948,6 @@ spa_stats_init(spa_t *spa) spa_read_history_init(spa); spa_txg_history_init(spa); spa_tx_assign_init(spa); - spa_io_history_init(spa); spa_mmp_history_init(spa); spa_state_init(spa); spa_iostats_init(spa); @@ -1010,7 +961,6 @@ spa_stats_destroy(spa_t *spa) spa_tx_assign_destroy(spa); spa_txg_history_destroy(spa); spa_read_history_destroy(spa); - spa_io_history_destroy(spa); spa_mmp_history_destroy(spa); } diff --git a/module/zfs/vdev_queue.c b/module/zfs/vdev_queue.c index 25a4bc69cc..198861edb8 100644 --- a/module/zfs/vdev_queue.c +++ b/module/zfs/vdev_queue.c @@ -35,8 +35,6 @@ #include #include #include -#include -#include #include /* @@ -516,35 +514,17 @@ vdev_queue_fini(vdev_t *vd) static void vdev_queue_io_add(vdev_queue_t *vq, zio_t *zio) { - spa_t *spa = zio->io_spa; - spa_history_kstat_t *shk = &spa->spa_stats.io_history; - ASSERT3U(zio->io_priority, <, ZIO_PRIORITY_NUM_QUEUEABLE); avl_add(vdev_queue_class_tree(vq, zio->io_priority), zio); avl_add(vdev_queue_type_tree(vq, zio->io_type), zio); - - if (shk->kstat != NULL) { - mutex_enter(&shk->lock); - kstat_waitq_enter(shk->kstat->ks_data); - mutex_exit(&shk->lock); - } } static void vdev_queue_io_remove(vdev_queue_t *vq, zio_t *zio) { - spa_t *spa = zio->io_spa; - spa_history_kstat_t *shk = &spa->spa_stats.io_history; - ASSERT3U(zio->io_priority, <, ZIO_PRIORITY_NUM_QUEUEABLE); avl_remove(vdev_queue_class_tree(vq, zio->io_priority), zio); avl_remove(vdev_queue_type_tree(vq, zio->io_type), zio); - - if (shk->kstat != NULL) { - mutex_enter(&shk->lock); - kstat_waitq_exit(shk->kstat->ks_data); - mutex_exit(&shk->lock); - } } static boolean_t @@ -564,9 +544,6 @@ vdev_queue_is_interactive(zio_priority_t p) static void vdev_queue_pending_add(vdev_queue_t *vq, zio_t *zio) { - spa_t *spa = zio->io_spa; - spa_history_kstat_t *shk = &spa->spa_stats.io_history; - ASSERT(MUTEX_HELD(&vq->vq_lock)); ASSERT3U(zio->io_priority, <, ZIO_PRIORITY_NUM_QUEUEABLE); vq->vq_class[zio->io_priority].vqc_active++; @@ -577,20 +554,11 @@ vdev_queue_pending_add(vdev_queue_t *vq, zio_t *zio) vq->vq_nia_credit--; } avl_add(&vq->vq_active_tree, zio); - - if (shk->kstat != NULL) { - mutex_enter(&shk->lock); - kstat_runq_enter(shk->kstat->ks_data); - mutex_exit(&shk->lock); - } } static void vdev_queue_pending_remove(vdev_queue_t *vq, zio_t *zio) { - spa_t *spa = zio->io_spa; - spa_history_kstat_t *shk = &spa->spa_stats.io_history; - ASSERT(MUTEX_HELD(&vq->vq_lock)); ASSERT3U(zio->io_priority, <, ZIO_PRIORITY_NUM_QUEUEABLE); vq->vq_class[zio->io_priority].vqc_active--; @@ -602,21 +570,6 @@ vdev_queue_pending_remove(vdev_queue_t *vq, zio_t *zio) } else if (vq->vq_ia_active == 0) vq->vq_nia_credit++; avl_remove(&vq->vq_active_tree, zio); - - if (shk->kstat != NULL) { - kstat_io_t *ksio = shk->kstat->ks_data; - - mutex_enter(&shk->lock); - kstat_runq_exit(ksio); - if (zio->io_type == ZIO_TYPE_READ) { - ksio->reads++; - ksio->nread += zio->io_size; - } else if (zio->io_type == ZIO_TYPE_WRITE) { - ksio->writes++; - ksio->nwritten += zio->io_size; - } - mutex_exit(&shk->lock); - } } static void