Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

Fixing gang ABD child removal race condition 

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. When removing a child from a
gang ABD we must acquire the child's abd_mtx to make sure that the
same ABD is not being added to another gang ABD while it is being
removed from a gang ABD. This fixes a race condition when checking
if an ABDs link is already active and part of another gang ABD before
adding it to a gang.

Added additional debug code for the gang ABD in abd_verify() to make
sure each child ABD has active links. Also check to make sure another
gang ABD is not added to a gang ABD.

Reviewed-by: Serapheim Dimitropoulos <serapheim@delphix.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Matt Ahrens <matt@delphix.com>
Signed-off-by: Brian Atkinson <batkinson@lanl.gov>
Closes #10511
This commit is contained in:
Brian Atkinson 2020-07-14 12:04:35 -06:00 committed by GitHub
parent c15d36c674
commit e4d3d77684
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 20 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
include/os/linux/spl/sys/list.h
View File

@ -26,6 +26,7 @@
#define _SPL_LIST_H
#include <sys/types.h>
#include <sys/debug.h>
#include <linux/list.h>
/*
@ -184,7 +185,8 @@ list_prev(list_t *list, void *object)
static inline int
list_link_active(list_node_t *node)
{
return (node->next != LIST_POISON1) && (node->prev != LIST_POISON2);
EQUIV(node->next == LIST_POISON1, node->prev == LIST_POISON2);
return (node->next != LIST_POISON1);
}
static inline void

1
lib/libspl/list.c
View File

@ -232,6 +232,7 @@ list_link_init(list_node_t *ln)
int
list_link_active(list_node_t *ln)
{
EQUIV(ln->next == NULL, ln->prev == NULL);
return (ln->next != NULL);
}

1
module/os/freebsd/spl/list.c
View File

@ -235,6 +235,7 @@ list_link_init(list_node_t *link)
int
list_link_active(list_node_t *link)
{
EQUIV(link->list_next == NULL, link->list_prev == NULL);
return (link->list_next != NULL);
}

18
module/zfs/abd.c
View File

@ -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 {
@ -290,10 +291,19 @@ static void
abd_free_gang_abd(abd_t *abd)
{
ASSERT(abd_is_gang(abd));
abd_t *cabd;
abd_t *cabd = list_head(&ABD_GANG(abd).abd_gang_chain);
while ((cabd = list_remove_head(&ABD_GANG(abd).abd_gang_chain))
!= NULL) {
while (cabd != NULL) {
/*
* We must acquire the child ABDs mutex to ensure that if it
* is being added to another gang ABD we will set the link
* as inactive when removing it from this gang ABD and before
* adding it to the other gang ABD.
*/
mutex_enter(&cabd->abd_mtx);
ASSERT(list_link_active(&cabd->abd_gang_link));
list_remove(&ABD_GANG(abd).abd_gang_chain, cabd);
mutex_exit(&cabd->abd_mtx);
abd->abd_size -= cabd->abd_size;
if (cabd->abd_flags & ABD_FLAG_GANG_FREE) {
if (cabd->abd_flags & ABD_FLAG_OWNER)
@ -301,6 +311,7 @@ abd_free_gang_abd(abd_t *abd)
else
abd_put(cabd);
}
cabd = list_head(&ABD_GANG(abd).abd_gang_chain);
}
ASSERT0(abd->abd_size);
list_destroy(&ABD_GANG(abd).abd_gang_chain);
@ -373,6 +384,7 @@ void
abd_gang_add(abd_t *pabd, abd_t *cabd, boolean_t free_on_free)
{
ASSERT(abd_is_gang(pabd));
ASSERT(!abd_is_gang(cabd));
abd_t *child_abd = NULL;
/*