From 0c4896de597fbd49f4d111d512555a902825b520 Mon Sep 17 00:00:00 2001 From: MigeljanImeri Date: Wed, 1 Nov 2023 11:13:33 -0600 Subject: [PATCH] Fix accounting error for pending sync IO ops in zpool iostat Currently vdev_queue_class_length is responsible for checking how long the queue length is, however, it doesn't check the length when a list is used, rather it just returns whether it is empty or not. To fix this I added a counter variable to vdev_queue_class to keep track of the sync IO ops, and changed vdev_queue_class_length to reference this variable instead. Along with this I removed the list_size field from the list implementations as it was only being used in few ASSERTS. It also allows us to make these changes without increasing the maximum size of vdev_queue_class. Signed-off-by: MigeljanImeri --- include/os/freebsd/spl/sys/list_impl.h | 1 - include/os/linux/spl/sys/list.h | 4 ++-- include/sys/vdev_impl.h | 5 ++++- lib/libspl/include/sys/list_impl.h | 3 +-- lib/libspl/list.c | 6 ++---- module/os/freebsd/spl/list.c | 5 ++--- module/zfs/vdev_queue.c | 7 +++++-- 7 files changed, 16 insertions(+), 15 deletions(-) diff --git a/include/os/freebsd/spl/sys/list_impl.h b/include/os/freebsd/spl/sys/list_impl.h index 09b70232e8ee..06a5c6d1dbc6 100644 --- a/include/os/freebsd/spl/sys/list_impl.h +++ b/include/os/freebsd/spl/sys/list_impl.h @@ -39,7 +39,6 @@ struct list_node { }; struct list { - size_t list_size; size_t list_offset; struct list_node list_head; }; diff --git a/include/os/linux/spl/sys/list.h b/include/os/linux/spl/sys/list.h index 80300df15abe..046a75e19353 100644 --- a/include/os/linux/spl/sys/list.h +++ b/include/os/linux/spl/sys/list.h @@ -48,7 +48,6 @@ typedef struct list_head list_node_t; typedef struct list { - size_t list_size; size_t list_offset; list_node_t list_head; } list_t; @@ -72,7 +71,8 @@ list_link_init(list_node_t *node) static inline void list_create(list_t *list, size_t size, size_t offset) { - list->list_size = size; + (void) size; + list->list_offset = offset; INIT_LIST_HEAD(&list->list_head); } diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h index ad9dc3aefd8e..3f2312c23438 100644 --- a/include/sys/vdev_impl.h +++ b/include/sys/vdev_impl.h @@ -131,7 +131,10 @@ typedef const struct vdev_ops { * Virtual device properties */ typedef union vdev_queue_class { - list_t vqc_list; + struct { + ulong_t vqc_list_numnodes; + list_t vqc_list; + }; avl_tree_t vqc_tree; } vdev_queue_class_t; diff --git a/lib/libspl/include/sys/list_impl.h b/lib/libspl/include/sys/list_impl.h index 24c1ceb2a9fa..5729795b7bcf 100644 --- a/lib/libspl/include/sys/list_impl.h +++ b/lib/libspl/include/sys/list_impl.h @@ -39,8 +39,7 @@ struct list_node { }; struct list { - size_t list_size; - size_t list_offset; + size_t list_offset; struct list_node list_head; }; diff --git a/lib/libspl/list.c b/lib/libspl/list.c index 24403698627c..a79da72cbd9e 100644 --- a/lib/libspl/list.c +++ b/lib/libspl/list.c @@ -62,10 +62,9 @@ void list_create(list_t *list, size_t size, size_t offset) { ASSERT(list); - ASSERT(size > 0); - ASSERT(size >= offset + sizeof (list_node_t)); - list->list_size = size; + (void) size; + list->list_offset = offset; list->list_head.next = list->list_head.prev = &list->list_head; } @@ -194,7 +193,6 @@ list_move_tail(list_t *dst, list_t *src) list_node_t *dstnode = &dst->list_head; list_node_t *srcnode = &src->list_head; - ASSERT(dst->list_size == src->list_size); ASSERT(dst->list_offset == src->list_offset); if (list_empty(src)) diff --git a/module/os/freebsd/spl/list.c b/module/os/freebsd/spl/list.c index ab6049cfbd43..94b09589cde9 100644 --- a/module/os/freebsd/spl/list.c +++ b/module/os/freebsd/spl/list.c @@ -62,9 +62,9 @@ void list_create(list_t *list, size_t size, size_t offset) { ASSERT3P(list, !=, NULL); - ASSERT3U(size, >=, offset + sizeof (list_node_t)); - list->list_size = size; + (void) size; + list->list_offset = offset; list->list_head.list_next = list->list_head.list_prev = &list->list_head; @@ -194,7 +194,6 @@ list_move_tail(list_t *dst, list_t *src) list_node_t *dstnode = &dst->list_head; list_node_t *srcnode = &src->list_head; - ASSERT3U(dst->list_size, ==, src->list_size); ASSERT3U(dst->list_offset, ==, src->list_offset); if (list_empty(src)) diff --git a/module/zfs/vdev_queue.c b/module/zfs/vdev_queue.c index 08d918467d03..092b3f375be0 100644 --- a/module/zfs/vdev_queue.c +++ b/module/zfs/vdev_queue.c @@ -273,8 +273,10 @@ vdev_queue_class_add(vdev_queue_t *vq, zio_t *zio) { zio_priority_t p = zio->io_priority; vq->vq_cqueued |= 1U << p; - if (vdev_queue_class_fifo(p)) + if (vdev_queue_class_fifo(p)) { list_insert_tail(&vq->vq_class[p].vqc_list, zio); + vq->vq_class[p].vqc_list_numnodes++; + } else avl_add(&vq->vq_class[p].vqc_tree, zio); } @@ -288,6 +290,7 @@ vdev_queue_class_remove(vdev_queue_t *vq, zio_t *zio) list_t *list = &vq->vq_class[p].vqc_list; list_remove(list, zio); empty = list_is_empty(list); + vq->vq_class[p].vqc_list_numnodes--; } else { avl_tree_t *tree = &vq->vq_class[p].vqc_tree; avl_remove(tree, zio); @@ -1069,7 +1072,7 @@ vdev_queue_class_length(vdev_t *vd, zio_priority_t p) { vdev_queue_t *vq = &vd->vdev_queue; if (vdev_queue_class_fifo(p)) - return (list_is_empty(&vq->vq_class[p].vqc_list) == 0); + return (vq->vq_class[p].vqc_list_numnodes); else return (avl_numnodes(&vq->vq_class[p].vqc_tree)); }