Skip to content

Commit

Permalink
Fix accounting error for pending sync IO ops in zpool iostat
Browse files Browse the repository at this point in the history
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 <ImeriMigel@gmail.com>
  • Loading branch information
MigeljanImeri committed Nov 3, 2023
1 parent 57b4098 commit 0c4896d
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 15 deletions.
1 change: 0 additions & 1 deletion include/os/freebsd/spl/sys/list_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ struct list_node {
};

struct list {
size_t list_size;
size_t list_offset;
struct list_node list_head;
};
Expand Down
4 changes: 2 additions & 2 deletions include/os/linux/spl/sys/list.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
5 changes: 4 additions & 1 deletion include/sys/vdev_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
3 changes: 1 addition & 2 deletions lib/libspl/include/sys/list_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
6 changes: 2 additions & 4 deletions lib/libspl/list.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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))
Expand Down
5 changes: 2 additions & 3 deletions module/os/freebsd/spl/list.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down
7 changes: 5 additions & 2 deletions module/zfs/vdev_queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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);
Expand Down Expand Up @@ -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));
}
Expand Down

0 comments on commit 0c4896d

Please sign in to comment.