Date: Thu, 25 Jun 2026 19:34:58 +0000 From: Alexander Motin <mav@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 6aaaf7ba4bba - main - zfsd: Improve spare selection Message-ID: <6a3d82e2.43d69.2227b890@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by mav: URL: https://cgit.FreeBSD.org/src/commit/?id=6aaaf7ba4bba5e01008924a61261b43a8356f591 commit 6aaaf7ba4bba5e01008924a61261b43a8356f591 Author: Alexander Motin <mav@FreeBSD.org> AuthorDate: 2026-06-25 19:33:59 +0000 Commit: Alexander Motin <mav@FreeBSD.org> CommitDate: 2026-06-25 19:34:43 +0000 zfsd: Improve spare selection Port OpenZFS PRs #18597 and #18578 from zed to zfsd. When activating a spare, sort candidates before trying them: 1. Distributed dRAID spare matching the failed vdev's group (fastest rebuild via sequential resilver) 2. Regular spares 3. Non-matching distributed spares (kernel will reject anyway) 4. Within each tier: prefer rotational match, then smallest sufficient size Also try all healthy spares in order rather than stopping at the first one, and use sequential rebuild for distributed spares. --- cddl/usr.sbin/zfsd/case_file.cc | 189 ++++++++++++++++++++++++++++++++-------- 1 file changed, 155 insertions(+), 34 deletions(-) diff --git a/cddl/usr.sbin/zfsd/case_file.cc b/cddl/usr.sbin/zfsd/case_file.cc index 852767aeb227..4f4f4c6ccf2a 100644 --- a/cddl/usr.sbin/zfsd/case_file.cc +++ b/cddl/usr.sbin/zfsd/case_file.cc @@ -59,6 +59,7 @@ #include <list> #include <map> #include <string> +#include <vector> #include <devdctl/guid.h> #include <devdctl/event.h> @@ -523,13 +524,82 @@ find_parent(nvlist_t *pool_config, nvlist_t *config, DevdCtl::Guid child_guid) return (NULL); } +/* + * Returns true if spare 'a' should be tried before spare 'b' when + * replacing a failed vdev with the given characteristics. + * + * Ordering criteria (most to least significant): + * 1. Distributed spare matching the failed vdev's dRAID is preferred + * most (distributed spares rebuild faster than traditional spares). + * Regular spares (no TOP_GUID) come next. Non-matching distributed + * spares are tried last, as the kernel will reject them anyway. + * 2. Matching rotational is preferred over mismatching. + * 3. Large enough is preferred over too small. + * 4. Smaller size is preferred over bigger (best fit). + */ +static bool +spare_is_preferred(nvlist_t *a, nvlist_t *b, bool have_rotational, + uint64_t vdev_rotational, uint64_t vdev_size, uint64_t top_guid) +{ + uint64_t a_top, b_top, a_rotational, b_rotational; + uint64_t a_size, b_size; + uint64_t *nvlist_array; + int a_pri, b_pri; + vdev_stat_t *vs; + uint_t c; + bool a_ok, b_ok; + + a_top = b_top = 0; + (void) nvlist_lookup_uint64(a, ZPOOL_CONFIG_TOP_GUID, &a_top); + (void) nvlist_lookup_uint64(b, ZPOOL_CONFIG_TOP_GUID, &b_top); + a_pri = (a_top == 0) ? 1 : + (a_top == top_guid || top_guid == 0) ? 2 : 0; + b_pri = (b_top == 0) ? 1 : + (b_top == top_guid || top_guid == 0) ? 2 : 0; + if (a_pri != b_pri) + return (a_pri > b_pri); + + if (have_rotational) { + a_rotational = b_rotational = 0; + (void) nvlist_lookup_uint64(a, + ZPOOL_CONFIG_VDEV_ROTATIONAL, &a_rotational); + (void) nvlist_lookup_uint64(b, + ZPOOL_CONFIG_VDEV_ROTATIONAL, &b_rotational); + if ((a_rotational == vdev_rotational) != + (b_rotational == vdev_rotational)) + return (a_rotational == vdev_rotational); + } + + a_size = b_size = 0; + if (nvlist_lookup_uint64_array(a, ZPOOL_CONFIG_VDEV_STATS, + &nvlist_array, &c) == 0) { + vs = reinterpret_cast<vdev_stat_t *>(nvlist_array); + a_size = vs->vs_rsize; + } + if (nvlist_lookup_uint64_array(b, ZPOOL_CONFIG_VDEV_STATS, + &nvlist_array, &c) == 0) { + vs = reinterpret_cast<vdev_stat_t *>(nvlist_array); + b_size = vs->vs_rsize; + } + a_ok = (a_size >= vdev_size); + b_ok = (b_size >= vdev_size); + if (a_ok != b_ok) + return (a_ok); + return (a_size < b_size); +} + bool CaseFile::ActivateSpare() { nvlist_t *config, *nvroot, *parent_config; - nvlist_t **spares; + nvlist_t *vdev_config, **spares, *spare; + uint64_t *nvlist_array; const char *devPath, *poolname, *vdev_type; - u_int nspares, i; - int error; + uint64_t vdev_rotational, vdev_size, top_guid; + vdev_stat_t *vs; + u_int nspares, i, key; + uint_t nstats; + int error, j; + bool have_vdev_rotational; ZpoolList zpl(ZpoolList::ZpoolByGUID, &m_poolGUID); zpool_handle_t *zhp(zpl.empty() ? NULL : zpl.front()); @@ -567,6 +637,16 @@ CaseFile::ActivateSpare() { return (false); } + /* + * Don't activate a spare if one is already working on this vdev. + */ + { + Vdev replaced(BeingReplacedBy(zhp)); + if (!replaced.DoesNotExist() && (replaced.IsResilvering() || + replaced.State() == VDEV_STATE_HEALTHY)) + return (false); + } + nspares = 0; nvlist_lookup_nvlist_array(nvroot, ZPOOL_CONFIG_SPARES, &spares, &nspares); @@ -576,49 +656,87 @@ CaseFile::ActivateSpare() { "No spares available for pool %s", poolname); return (false); } + + /* + * Collect the failed vdev's parameters for optimal spare selection. + */ + vdev_rotational = vdev_size = top_guid = 0; + have_vdev_rotational = false; + vdev_config = VdevIterator(zhp).Find(m_vdevGUID); + if (vdev_config != NULL) { + have_vdev_rotational = (nvlist_lookup_uint64(vdev_config, + ZPOOL_CONFIG_VDEV_ROTATIONAL, &vdev_rotational) == 0); + if (nvlist_lookup_uint64_array(vdev_config, + ZPOOL_CONFIG_VDEV_STATS, &nvlist_array, &nstats) == 0) { + vs = reinterpret_cast<vdev_stat_t *>(nvlist_array); + vdev_size = vs->vs_rsize; + } + (void) nvlist_lookup_uint64(vdev_config, + ZPOOL_CONFIG_TOP_GUID, &top_guid); + } + + /* + * Build a sorted index array over the spares, so that better + * candidates are tried first. + */ + std::vector<u_int> order(nspares); + for (i = 0; i < nspares; i++) + order[i] = i; + for (i = 1; i < nspares; i++) { + key = order[i]; + j = (int)i - 1; + while (j >= 0 && spare_is_preferred(spares[key], + spares[order[j]], have_vdev_rotational, vdev_rotational, + vdev_size, top_guid)) { + order[j + 1] = order[j]; + j--; + } + order[j + 1] = key; + } + + /* + * Try each spare in sorted order until one succeeds. + */ for (i = 0; i < nspares; i++) { - uint64_t *nvlist_array; - vdev_stat_t *vs; - uint_t nstats; + spare = spares[order[i]]; - if (nvlist_lookup_uint64_array(spares[i], + if (nvlist_lookup_uint64_array(spare, ZPOOL_CONFIG_VDEV_STATS, &nvlist_array, &nstats) != 0) { syslog(LOG_ERR, "CaseFile::ActivateSpare: Could not " "find vdev stats for pool %s, spare %d", - poolname, i); - return (false); + poolname, order[i]); + continue; } vs = reinterpret_cast<vdev_stat_t *>(nvlist_array); - if ((vs->vs_aux != VDEV_AUX_SPARED) - && (vs->vs_state == VDEV_STATE_HEALTHY)) { - /* We found a usable spare */ - break; - } - } + if ((vs->vs_aux == VDEV_AUX_SPARED) + || (vs->vs_state != VDEV_STATE_HEALTHY)) + continue; - if (i == nspares) { - /* No available spares were found */ - return (false); - } + error = nvlist_lookup_string(spare, ZPOOL_CONFIG_PATH, + &devPath); + if (error != 0) { + syslog(LOG_ERR, "CaseFile::ActivateSpare: Cannot " + "determine the path of pool %s, spare %d. " + "Error %d", poolname, order[i], error); + continue; + } - error = nvlist_lookup_string(spares[i], ZPOOL_CONFIG_PATH, &devPath); - if (error != 0) { - syslog(LOG_ERR, "CaseFile::ActivateSpare: Cannot determine " - "the path of pool %s, spare %d. Error %d", - poolname, i, error); - return (false); - } + error = nvlist_lookup_string(spare, ZPOOL_CONFIG_TYPE, + &vdev_type); + if (error != 0) { + syslog(LOG_ERR, "CaseFile::ActivateSpare: Cannot " + "determine the vdev type of pool %s, " + "spare %d. Error %d", + poolname, order[i], error); + continue; + } - error = nvlist_lookup_string(spares[i], ZPOOL_CONFIG_TYPE, &vdev_type); - if (error != 0) { - syslog(LOG_ERR, "CaseFile::ActivateSpare: Cannot determine " - "the vdev type of pool %s, spare %d. Error %d", - poolname, i, error); - return (false); + if (Replace(vdev_type, devPath, /*isspare*/true)) + return (true); } - return (Replace(vdev_type, devPath, /*isspare*/true)); + return (false); } /* Does the argument event refer to a checksum error? */ @@ -1203,8 +1321,11 @@ CaseFile::Replace(const char* vdev_type, const char* path, bool isspare) { /* Data was copied when added to the root vdev. */ nvlist_free(newvd); + /* Prefer sequential resilvering for distributed spares. */ retval = (zpool_vdev_attach(zhp, oldstr.c_str(), path, nvroot, - /*replace*/B_TRUE, /*rebuild*/ B_FALSE) == 0); + /*replace*/B_TRUE, + strcmp(vdev_type, VDEV_TYPE_DRAID_SPARE) == 0 ? + B_TRUE : B_FALSE) == 0); if (retval) syslog(LOG_INFO, "Replacing vdev(%s/%s) with %s\n", poolname, oldstr.c_str(), path);home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a3d82e2.43d69.2227b890>