Skip to content

Pool split leaks DTL spacemap objects #18808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tsoome wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Pool split leaks DTL spacemap objects #18808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d4844a6
Pool split leaks DTL spacemap objects
tsoome Jul 15, 2026
413c35e
remove vdev_zaps_007_pos from zts-report.py.in
tsoome Jul 23, 2026
4e6348e
zpool_split_devices: use zdb -MC to test split pools
tsoome Jul 23, 2026
653666c
Allow vdev_count_leaves() to be called with lock held
tsoome Jul 31, 2026
4a5d619
zpool split does leak DTLs in split off pool
tsoome Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 103 additions & 3 deletions module/zfs/spa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9135,6 +9135,46 @@ spa_vdev_trim(spa_t *spa, nvlist_t *nv, uint64_t cmd_type, uint64_t rate,
return (total_errors);
}

typedef struct spa_split_dtl_arg {
spa_t *ssda_spa; /* the new pool */
uint64_t *ssda_objs; /* original DTL space map objects */
uint_t ssda_count; /* nitems in ssda_objs */
} spa_split_dtl_arg_t;

/*
* Record the DTL space map object of every leaf that has one into objs[],
* advancing *idxp. These are the objects that will be carried, via the
* copied MOS, onto the split disks.
*/
static void
spa_split_collect_dtl(vdev_t *vd, uint64_t *objs, uint_t *idxp)
{
if (vd->vdev_ops->vdev_op_leaf) {
if (vd->vdev_dtl_sm != NULL)
objs[(*idxp)++] = space_map_object(vd->vdev_dtl_sm);
return;
}
for (uint64_t c = 0; c < vd->vdev_children; c++)
spa_split_collect_dtl(vd->vdev_child[c], objs, idxp);
}

/*
* Callback that frees the inherited DTL space map objects from the new
* pool MOS. The new pool MOS is a byte copy of the original pool, so it
* contains a DTL space map object for every leaf of the original pool.
* The new pool references none of them because split leaves
* start with an empty DTL and allocate their own on demand.
*/
static void
spa_split_dtl_free_sync(void *arg, dmu_tx_t *tx)
{
spa_split_dtl_arg_t *ssda = arg;
objset_t *mos = ssda->ssda_spa->spa_meta_objset;

for (uint_t i = 0; i < ssda->ssda_count; i++)
space_map_free_obj(mos, ssda->ssda_objs[i], tx);
}

/*
* Split a set of devices from their mirrors, and create a new pool from them.
*/
Expand All @@ -9149,7 +9189,9 @@ spa_vdev_split_mirror(spa_t *spa, const char *newname, nvlist_t *config,
nvlist_t **child, *nvl, *tmp;
dmu_tx_t *tx;
const char *altroot = NULL;
vdev_t *rvd, **vml = NULL; /* vdev modify list */
vdev_t *rvd, **vml = NULL; /* vdev modify list */
uint64_t *dtl_objs = NULL; /* DTL objs from original pool */
uint_t ndtl = 0, nleaves;
boolean_t activate_slog;

ASSERT(spa_writeable(spa));
Expand Down Expand Up @@ -9297,6 +9339,11 @@ spa_vdev_split_mirror(spa_t *spa, const char *newname, nvlist_t *config,
return (spa_vdev_exit(spa, NULL, txg, error));
}

/* Create array of DTL objects. */
nleaves = vdev_count_leaves(spa);
dtl_objs = kmem_zalloc(nleaves * sizeof (uint64_t), KM_SLEEP);
spa_split_collect_dtl(spa->spa_root_vdev, dtl_objs, &ndtl);

/* stop writers from using the disks */
for (c = 0; c < children; c++) {
if (vml[c] != NULL)
Expand Down Expand Up @@ -9394,6 +9441,20 @@ spa_vdev_split_mirror(spa_t *spa, const char *newname, nvlist_t *config,
B_TRUE));
}

/*
* Free the DTL space map objects inherited from the original pool
* MOS so we wont leak them.
*/
if (ndtl != 0) {
spa_split_dtl_arg_t ssda;

ssda.ssda_spa = newspa;
ssda.ssda_objs = dtl_objs;
ssda.ssda_count = ndtl;
VERIFY0(dsl_sync_task(spa_name(newspa), NULL,
spa_split_dtl_free_sync, &ssda, 0, ZFS_SPACE_CHECK_NONE));
}

/* set the props */
if (props != NULL) {
spa_configfile_set(newspa, props, B_FALSE);
Expand Down Expand Up @@ -9433,11 +9494,33 @@ spa_vdev_split_mirror(spa_t *spa, const char *newname, nvlist_t *config,
}

vdev_split(vml[c]);

/*
* As in spa_vdev_detach(), mark the vdev detached
* and dirty its DTL, so that vdev_dtl_sync() frees
* the leaf's DTL space map object.
*/
vml[c]->vdev_detached = B_TRUE;

/*
* The leaf ZAP was transferred to the new pool
* and this pool's copy is destroyed by the AVZ
* rebuild below, so clear it to keep
* vdev_dtl_sync() from destroying it again.
*/
vml[c]->vdev_leaf_zap = 0;

/*
* vml[c]->vdev_top may be stale; the
* surviving top-level vdev is rvd->vdev_child[c].
*/
if (vml[c]->vdev_dtl_sm != NULL)
vdev_dirty(rvd->vdev_child[c], VDD_DTL,
vml[c], txg);

Comment on lines +9498 to +9520
if (error == 0)
spa_history_log_internal(spa, "detach", tx,
"vdev=%s", vml[c]->vdev_path);

vdev_free(vml[c]);
}
}
spa->spa_avz_action = AVZ_ACTION_REBUILD;
Expand All @@ -9448,6 +9531,18 @@ spa_vdev_split_mirror(spa_t *spa, const char *newname, nvlist_t *config,
dmu_tx_commit(tx);
(void) spa_vdev_exit(spa, NULL, txg, 0);

/*
* txg is synced, free vdevs.
*/
spa_config_enter(spa, SCL_STATE_ALL, spa, RW_WRITER);
for (c = 0; c < children; c++) {
if (vml[c] != NULL && vml[c]->vdev_ops != &vdev_indirect_ops) {
ASSERT0P(vml[c]->vdev_dtl_sm);
vdev_free(vml[c]);
}
}
spa_config_exit(spa, SCL_STATE_ALL, spa);

if (zio_injection_enabled)
zio_handle_panic_injection(spa, FTAG, 3);

Expand All @@ -9457,6 +9552,8 @@ spa_vdev_split_mirror(spa_t *spa, const char *newname, nvlist_t *config,

newspa->spa_is_splitting = B_FALSE;
kmem_free(vml, children * sizeof (vdev_t *));
if (dtl_objs != NULL)
kmem_free(dtl_objs, nleaves * sizeof (uint64_t));

/* if we're not going to mount the filesystems in userland, export */
if (exp)
Expand Down Expand Up @@ -9490,6 +9587,9 @@ spa_vdev_split_mirror(spa_t *spa, const char *newname, nvlist_t *config,
(void) spa_vdev_exit(spa, NULL, txg, error);

kmem_free(vml, children * sizeof (vdev_t *));
if (dtl_objs != NULL)
kmem_free(dtl_objs, nleaves * sizeof (uint64_t));

return (error);
}

Expand Down
9 changes: 7 additions & 2 deletions module/zfs/vdev.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,10 +564,15 @@ int
vdev_count_leaves(spa_t *spa)
{
int rc;
boolean_t held;

spa_config_enter(spa, SCL_VDEV, FTAG, RW_READER);
held = (spa_config_held(spa, SCL_VDEV, RW_WRITER) == SCL_VDEV);

if (!held)
spa_config_enter(spa, SCL_VDEV, FTAG, RW_READER);
rc = vdev_count_leaves_impl(spa->spa_root_vdev);
spa_config_exit(spa, SCL_VDEV, FTAG);
if (!held)
spa_config_exit(spa, SCL_VDEV, FTAG);

return (rc);
}
Expand Down
1 change: 0 additions & 1 deletion tests/test-runner/bin/zts-report.py.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,6 @@ known = {
'refreserv/refreserv_004_pos': ['FAIL', known_reason],
'rootpool/setup': ['SKIP', na_reason],
'rsend/rsend_008_pos': ['SKIP', 6066],
'vdev_zaps/vdev_zaps_007_pos': ['FAIL', known_reason],
}

if sys.platform.startswith('freebsd'):
Expand Down
16 changes: 16 additions & 0 deletions tests/zfs-tests/tests/functional/cli_root/zpool_split/zpool_split_devices.ksh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,23 @@ do
typeset conf=${poolconfs[$i]}
setup_mirror $conf
log_mustnot zpool split $TESTPOOL $TESTPOOL2 ${baddevs[$i]}

# Force creating DTL objects on vdevs
typeset dtldev=${gooddevs[$i]%% *}
typeset mntpnt=$(get_prop mountpoint $TESTPOOL)
log_must zpool offline $TESTPOOL $dtldev
log_must dd if=/dev/zero of=$mntpnt/dtlfile bs=1k count=1
log_must zpool online $TESTPOOL $dtldev
sync_pool $TESTPOOL true

log_must zpool split -R $altroot $TESTPOOL $TESTPOOL2 ${gooddevs[$i]}
sync_pool $TESTPOOL true
sync_pool $TESTPOOL2 true

# check space maps
log_must zdb -MC $TESTPOOL
log_must zdb -MC $TESTPOOL2

@behlendorf behlendorf Aug 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, this check is reliably failing in the CI. If we trust the error message it seems the split pool doesn't get added to the cache file. That sure seems like another bug which has been hiding.

  11:41:07.28 failed to find config for pool 'testpool2': no matching pools
  11:41:07.28 zdb: can't open 'testpool2': No such file or directory
  11:41:07.28 ERROR: zdb -MC testpool2 exited 1

@tsoome tsoome Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my test machine, the cause was left around pool traces - like exported pool once created from multiple disks, then new pool was created on some of those disks and zdb got confused from those. When I made sure all disks had destroyed pools, the error went away... I did that test on illumos, havent checked with OpenZFS yet.


# Verify "good" devices ended up in the new pool
log_must poolexists $TESTPOOL2
for filedev in ${gooddevs[$i]}; do
Expand Down
Loading