Illumos #1951: leaking a vdev when removing an l2cache device

1952 memory leak when adding a file-based l2arc device
1954 leak in ZFS from metaslab_group_create and zfs_ereport_checksum

Reviewed by: Adam Leventhal <ahl@delphix.com>
Reviewed by: Matt Ahrens <matt@delphix.com>
Reviewed by: Eric Schrock <eric.schrock@delphix.com>
Reviewed by: Bill Pijewski <wdp@joyent.com>
Reviewed by: Dan McDonald <danmcd@nexenta.com>
Approved by: Eric Schrock <eric.schrock@delphix.com>

References to Illumos issues:
  https://www.illumos.org/issues/1951
  https://www.illumos.org/issues/1952
  https://www.illumos.org/issues/1954

Ported-by: Richard Yao <ryao@cs.stonybrook.edu>
Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Closes #650

diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h
index 161bd21f05a6eea1e7fd8ff04d307ab1692fac06..1df61a587d6cb25e9c004f72837ccbc74a6d8ff4 100644 (file)
--- a/include/sys/vdev_impl.h
+++ b/include/sys/vdev_impl.h
@@ -261,6 +261,7 @@ typedef struct vdev_label {
 #define        VDEV_ALLOC_L2CACHE      3
 #define        VDEV_ALLOC_ROOTPOOL     4
 #define        VDEV_ALLOC_SPLIT        5
+#define        VDEV_ALLOC_ATTACH       6
 
 /*
  * Allocate or free a vdev

diff --git a/module/zfs/spa.c b/module/zfs/spa.c
index c4a8418ef53f05522e7e62b7663b9aaf30e68087..a43b88338b4f021863444eb491358c07a8a35e9d 100644 (file)
--- a/module/zfs/spa.c
+++ b/module/zfs/spa.c
@@ -1007,8 +1007,10 @@ spa_unload(spa_t *spa)
        }
        spa->spa_spares.sav_count = 0;
 
-       for (i = 0; i < spa->spa_l2cache.sav_count; i++)
+       for (i = 0; i < spa->spa_l2cache.sav_count; i++) {
+               vdev_clear_stats(spa->spa_l2cache.sav_vdevs[i]);
                vdev_free(spa->spa_l2cache.sav_vdevs[i]);
+       }
        if (spa->spa_l2cache.sav_vdevs) {
                kmem_free(spa->spa_l2cache.sav_vdevs,
                    spa->spa_l2cache.sav_count * sizeof (void *));
@@ -1231,11 +1233,13 @@ spa_load_l2cache(spa_t *spa)
 
                vd = oldvdevs[i];
                if (vd != NULL) {
+                       ASSERT(vd->vdev_isl2cache);
+
                        if (spa_l2cache_exists(vd->vdev_guid, &pool) &&
                            pool != 0ULL && l2arc_vdev_present(vd))
                                l2arc_remove_vdev(vd);
-                       (void) vdev_close(vd);
-                       spa_l2cache_remove(vd);
+                       vdev_clear_stats(vd);
+                       vdev_free(vd);
                }
        }
 
@@ -2743,6 +2747,7 @@ spa_validate_aux_devs(spa_t *spa, nvlist_t *nvroot, uint64_t crtxg, int mode,
                if ((strcmp(config, ZPOOL_CONFIG_L2CACHE) == 0) &&
                    strcmp(vd->vdev_ops->vdev_op_type, VDEV_TYPE_DISK) != 0) {
                        error = ENOTBLK;
+                       vdev_free(vd);
                        goto out;
                }
 #endif
@@ -2852,10 +2857,6 @@ spa_l2cache_drop(spa_t *spa)
                if (spa_l2cache_exists(vd->vdev_guid, &pool) &&
                    pool != 0ULL && l2arc_vdev_present(vd))
                        l2arc_remove_vdev(vd);
-               if (vd->vdev_isl2cache)
-                       spa_l2cache_remove(vd);
-               vdev_clear_stats(vd);
-               (void) vdev_close(vd);
        }
 }
 
@@ -3851,7 +3852,7 @@ spa_vdev_attach(spa_t *spa, uint64_t guid, nvlist_t *nvroot, int replacing)
        pvd = oldvd->vdev_parent;
 
        if ((error = spa_config_parse(spa, &newrootvd, nvroot, NULL, 0,
-           VDEV_ALLOC_ADD)) != 0)
+           VDEV_ALLOC_ATTACH)) != 0)
                return (spa_vdev_exit(spa, NULL, txg, EINVAL));
 
        if (newrootvd->vdev_children != 1)

diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c
index 9f044b68c1d8fd656d15b42f4df9fd935029b918..1630d2fae69dd490c86380d835e2f05260d3522a 100644 (file)
--- a/module/zfs/vdev.c
+++ b/module/zfs/vdev.c
@@ -492,7 +492,7 @@ vdev_alloc(spa_t *spa, vdev_t **vdp, nvlist_t *nv, vdev_t *parent, uint_t id,
                    &vd->vdev_removing);
        }
 
-       if (parent && !parent->vdev_parent) {
+       if (parent && !parent->vdev_parent && alloctype != VDEV_ALLOC_ATTACH) {
                ASSERT(alloctype == VDEV_ALLOC_LOAD ||
                    alloctype == VDEV_ALLOC_ADD ||
                    alloctype == VDEV_ALLOC_SPLIT ||
@@ -669,6 +669,8 @@ vdev_top_transfer(vdev_t *svd, vdev_t *tvd)
        svd->vdev_ms_shift = 0;
        svd->vdev_ms_count = 0;
 
+       if (tvd->vdev_mg)
+               ASSERT3P(tvd->vdev_mg, ==, svd->vdev_mg);
        tvd->vdev_mg = svd->vdev_mg;
        tvd->vdev_ms = svd->vdev_ms;
 

diff --git a/module/zfs/zfs_fm.c b/module/zfs/zfs_fm.c
index 74f2756e3b676b8de0324a3160f054c76037ac90..7801837f104b8166d09888c5c15e69eff147d192 100644 (file)
--- a/module/zfs/zfs_fm.c
+++ b/module/zfs/zfs_fm.c
@@ -23,6 +23,10 @@
  * Use is subject to license terms.
  */
 
+/*
+ * Copyright (c) 2012 by Delphix. All rights reserved.
+ */
+
 #include <sys/spa.h>
 #include <sys/spa_impl.h>
 #include <sys/vdev.h>
@@ -706,6 +710,10 @@ zfs_ereport_start_checksum(spa_t *spa, vdev_t *vd,
 
        if (report->zcr_ereport == NULL) {
                report->zcr_free(report->zcr_cbdata, report->zcr_cbinfo);
+               if (report->zcr_ckinfo != NULL) {
+                       kmem_free(report->zcr_ckinfo,
+                           sizeof (*report->zcr_ckinfo));
+               }
                kmem_free(report, sizeof (*report));
                return;
        }