From 5ffb9d1d05d7c512b987dff51f587466d537770f Mon Sep 17 00:00:00 2001 From: George Wilson Date: Sun, 8 Apr 2012 13:23:08 -0400 Subject: [PATCH] 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 Reviewed by: Matt Ahrens Reviewed by: Eric Schrock Reviewed by: Bill Pijewski Reviewed by: Dan McDonald Approved by: Eric Schrock 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 Signed-off-by: Brian Behlendorf Closes #650 --- include/sys/vdev_impl.h | 1 + module/zfs/spa.c | 17 +++++++++-------- module/zfs/vdev.c | 4 +++- module/zfs/zfs_fm.c | 8 ++++++++ 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/include/sys/vdev_impl.h b/include/sys/vdev_impl.h index 161bd21f0..1df61a587 100644 --- 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 c4a8418ef..a43b88338 100644 --- 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 9f044b68c..1630d2fae 100644 --- 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 74f2756e3..7801837f1 100644 --- 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 #include #include @@ -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; }