From 4770aa0643a7fc62f81b2d60e4a46de4bfd1aa04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kan=20Johansson?= Date: Fri, 7 Oct 2016 22:25:35 +0200 Subject: [PATCH] Fix vdev_open_child() race on updating vdev_parent->vdev_nonrot Updating vd->vdev_parent->vdev_nonrot in vdev_open_child() is a race when vdev_open_child is called for many children from a task queue. vdev_open_child() is only called by vdev_open_children(), let the latter update the parent vdev_nonrot member. The update was already there, so done twice previously. Thus using the same logic at the end in vdev_open_children() to update vdev_nonrot, either we are vdev_uses_zvols() or not. Reviewed-by: Richard Elling Reviewed-by: Brian Behlendorf Signed-off-by: Haakan T Johansson Closes #5162 --- module/zfs/vdev.c | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index dcf56d8df..104db3d15 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -1124,7 +1124,6 @@ vdev_open_child(void *arg) vd->vdev_open_thread = curthread; vd->vdev_open_error = vdev_open(vd); vd->vdev_open_thread = NULL; - vd->vdev_parent->vdev_nonrot &= vd->vdev_nonrot; } static boolean_t @@ -1151,29 +1150,27 @@ vdev_open_children(vdev_t *vd) int children = vd->vdev_children; int c; - vd->vdev_nonrot = B_TRUE; - /* * in order to handle pools on top of zvols, do the opens * in a single thread so that the same thread holds the * spa_namespace_lock */ if (vdev_uses_zvols(vd)) { - for (c = 0; c < children; c++) { + for (c = 0; c < children; c++) vd->vdev_child[c]->vdev_open_error = vdev_open(vd->vdev_child[c]); - vd->vdev_nonrot &= vd->vdev_child[c]->vdev_nonrot; - } - return; + } else { + tq = taskq_create("vdev_open", children, minclsyspri, + children, children, TASKQ_PREPOPULATE); + + for (c = 0; c < children; c++) + VERIFY(taskq_dispatch(tq, vdev_open_child, + vd->vdev_child[c], TQ_SLEEP) != 0); + + taskq_destroy(tq); } - tq = taskq_create("vdev_open", children, minclsyspri, - children, children, TASKQ_PREPOPULATE); - for (c = 0; c < children; c++) - VERIFY(taskq_dispatch(tq, vdev_open_child, vd->vdev_child[c], - TQ_SLEEP) != 0); - - taskq_destroy(tq); + vd->vdev_nonrot = B_TRUE; for (c = 0; c < children; c++) vd->vdev_nonrot &= vd->vdev_child[c]->vdev_nonrot;