Skip to content

Commit

Permalink
Additional gang ABD debug code
Browse files Browse the repository at this point in the history
Adding call to list_link_not_active() to avoid a logic short circuit on
the && so both prev and next are checked to see if they are pointing at
LIST_POISON values when free'ing the ABD struct. Also added check in
abd_verify() to make sure that ABD has active links when it is part of
a gang ABD.

On linux the list debug code has been setting off a failure when
checking that the node->next->prev value is pointing back at the node.
At times this check evaluates to 0xdead.

Signed-off-by: Brian Atkinson <batkinson@lanl.gov>
  • Loading branch information
bwatkinson committed Jul 2, 2020
1 parent 2ac6aa1 commit 5ae5354
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 2 deletions.
1 change: 1 addition & 0 deletions include/os/freebsd/spl/sys/list.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ int list_is_empty(list_t *);
void list_link_init(list_node_t *);
void list_link_replace(list_node_t *, list_node_t *);

int list_link_not_active(list_node_t *);
int list_link_active(list_node_t *);

#ifdef __cplusplus
Expand Down
6 changes: 6 additions & 0 deletions include/os/linux/spl/sys/list.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ list_prev(list_t *list, void *object)
return (NULL);
}

static inline int
list_link_not_active(list_node_t *node)
{
return ((node->next == LIST_POISON1) && (node->prev == LIST_POISON2));
}

static inline int
list_link_active(list_node_t *node)
{
Expand Down
1 change: 1 addition & 0 deletions lib/libspl/include/sys/list.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ int list_is_empty(list_t *);
void list_link_init(list_node_t *);
void list_link_replace(list_node_t *, list_node_t *);

int list_link_not_active(list_node_t *);
int list_link_active(list_node_t *);

#ifdef __cplusplus
Expand Down
6 changes: 6 additions & 0 deletions lib/libspl/list.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,12 @@ list_link_init(list_node_t *ln)
ln->prev = NULL;
}

int
list_link_not_active(list_node_t *in)
{
return ((in->next == NULL) && (in->prev == NULL));
}

int
list_link_active(list_node_t *ln)
{
Expand Down
6 changes: 6 additions & 0 deletions module/os/freebsd/spl/list.c
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,12 @@ list_link_active(list_node_t *link)
return (link->list_next != NULL);
}

int
list_link_not_active(list_node_t *link)
{
return (link->list_next == NULL && link->list_prev == NULL);
}

int
list_is_empty(list_t *list)
{
Expand Down
2 changes: 1 addition & 1 deletion module/os/freebsd/zfs/abd_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ abd_free_struct(abd_t *abd)
int size = MAX(sizeof (abd_t),
offsetof(abd_t, abd_u.abd_scatter.abd_chunks[chunkcnt]));
mutex_destroy(&abd->abd_mtx);
ASSERT(!list_link_active(&abd->abd_gang_link));
ASSERT(list_link_not_active(&abd->abd_gang_link));
kmem_free(abd, size);
ABDSTAT_INCR(abdstat_struct_size, -size);
}
Expand Down
2 changes: 1 addition & 1 deletion module/os/linux/zfs/abd_os.c
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ void
abd_free_struct(abd_t *abd)
{
mutex_destroy(&abd->abd_mtx);
ASSERT(!list_link_active(&abd->abd_gang_link));
ASSERT(list_link_not_active(&abd->abd_gang_link));
kmem_cache_free(abd_cache, abd);
ABDSTAT_INCR(abdstat_struct_size, -(int)sizeof (abd_t));
}
Expand Down
3 changes: 3 additions & 0 deletions module/zfs/abd.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ abd_verify(abd_t *abd)
for (abd_t *cabd = list_head(&ABD_GANG(abd).abd_gang_chain);
cabd != NULL;
cabd = list_next(&ABD_GANG(abd).abd_gang_chain, cabd)) {
ASSERT(list_link_active(&cabd->abd_gang_link));
abd_verify(cabd);
}
} else {
Expand Down Expand Up @@ -373,6 +374,7 @@ void
abd_gang_add(abd_t *pabd, abd_t *cabd, boolean_t free_on_free)
{
ASSERT(abd_is_gang(pabd));
ASSERT3B(abd_is_gang(cabd), ==, B_FALSE);
abd_t *child_abd = NULL;

/*
Expand Down Expand Up @@ -422,6 +424,7 @@ abd_gang_add(abd_t *pabd, abd_t *cabd, boolean_t free_on_free)
ASSERT3P(child_abd, !=, NULL);

list_insert_tail(&ABD_GANG(pabd).abd_gang_chain, child_abd);
ASSERT3B(list_link_active(&child_abd->abd_gang_link), ==, B_TRUE);
mutex_exit(&cabd->abd_mtx);
pabd->abd_size += child_abd->abd_size;
}
Expand Down

0 comments on commit 5ae5354

Please sign in to comment.