From f1c4323d66c83fb1d6f8a3977f4d922ee7f57a68 Mon Sep 17 00:00:00 2001 From: Don Brady Date: Wed, 26 Jun 2024 22:23:17 +0000 Subject: [PATCH] Guarantee that spa_load_guid is unique The zpool reguid feature introduced the spa_load_guid, which is a transient value used for runtime identification purposes in the ARC. This value is not the same as the spa's persistent pool guid. However, the value is seeded from spa_generate_load_guid() which does not check for uniqueness against the spa_load_guid from other pools. Although extremely rare, you can end up with two different pools sharing the same spa_load_guid value! This change guarantees that the value is always unique and additionally not still in use by an async arc flush task. Sponsored-by: Klara, Inc. Sponsored-by: Wasabi Technology, Inc. Signed-off-by: Don Brady --- include/sys/arc.h | 1 + include/sys/spa.h | 1 + module/zfs/arc.c | 127 +++++++++++++++++++++++++++++++++++++----- module/zfs/spa_misc.c | 28 ++++++++++ module/zfs/vdev.c | 2 +- 5 files changed, 143 insertions(+), 16 deletions(-) diff --git a/include/sys/arc.h b/include/sys/arc.h index 5c7474c4b070..154b3ca76618 100644 --- a/include/sys/arc.h +++ b/include/sys/arc.h @@ -322,6 +322,7 @@ void arc_flush(spa_t *spa, boolean_t retry); void arc_flush_async(spa_t *spa); void arc_tempreserve_clear(uint64_t reserve); int arc_tempreserve_space(spa_t *spa, uint64_t reserve, uint64_t txg); +boolean_t arc_async_flush_guid_inuse(uint64_t load_guid); uint64_t arc_all_memory(void); uint64_t arc_default_max(uint64_t min, uint64_t allmem); diff --git a/include/sys/spa.h b/include/sys/spa.h index 3073c4d1b937..7bfb3f558b75 100644 --- a/include/sys/spa.h +++ b/include/sys/spa.h @@ -1086,6 +1086,7 @@ extern boolean_t spa_guid_exists(uint64_t pool_guid, uint64_t device_guid); extern char *spa_strdup(const char *); extern void spa_strfree(char *); extern uint64_t spa_generate_guid(spa_t *spa); +extern uint64_t spa_generate_load_guid(void); extern void snprintf_blkptr(char *buf, size_t buflen, const blkptr_t *bp); extern void spa_freeze(spa_t *spa); extern int spa_change_guid(spa_t *spa); diff --git a/module/zfs/arc.c b/module/zfs/arc.c index d903656de29e..b629b40e0f3c 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -775,6 +775,21 @@ static buf_hash_table_t buf_hash_table; uint64_t zfs_crc64_table[256]; +/* + * Asynchronous ARC flush + * + * We track these in a list for arc_async_flush_guid_inuse() + */ +static list_t arc_async_flush_list; +static kmutex_t arc_async_flush_lock; + +typedef struct arc_async_flush { + uint64_t af_spa_guid; + taskqid_t af_task_id; + list_node_t af_node; +} arc_async_flush_t; + + /* * Level 2 ARC */ @@ -4409,19 +4424,50 @@ arc_flush(spa_t *spa, boolean_t retry) arc_flush_impl(spa != NULL ? spa_load_guid(spa) : 0, retry); } +static arc_async_flush_t * +arc_async_flush_add(uint64_t spa_guid, taskqid_t task_id) +{ + arc_async_flush_t *af = kmem_alloc(sizeof (*af), KM_SLEEP); + af->af_spa_guid = spa_guid; + af->af_task_id = task_id; + list_link_init(&af->af_node); + + mutex_enter(&arc_async_flush_lock); + list_insert_tail(&arc_async_flush_list, af); + mutex_exit(&arc_async_flush_lock); + + return (af); +} + +static void +arc_async_flush_remove(uint64_t spa_guid, taskqid_t task_id) +{ + mutex_enter(&arc_async_flush_lock); + for (arc_async_flush_t *af = list_head(&arc_async_flush_list); + af != NULL; af = list_next(&arc_async_flush_list, af)) { + if (af->af_spa_guid == spa_guid && af->af_task_id == task_id) { + list_remove(&arc_async_flush_list, af); + kmem_free(af, sizeof (*af)); + break; + } + } + mutex_exit(&arc_async_flush_lock); +} + static void arc_flush_task(void *arg) { - uint64_t guid = *((uint64_t *)arg); + arc_async_flush_t *af = arg; hrtime_t start_time = gethrtime(); + uint64_t spa_guid = af->af_spa_guid; - arc_flush_impl(guid, B_FALSE); - kmem_free(arg, sizeof (uint64_t *)); + arc_flush_impl(spa_guid, B_FALSE); + arc_async_flush_remove(spa_guid, af->af_task_id); uint64_t elaspsed = NSEC2MSEC(gethrtime() - start_time); if (elaspsed > 0) { zfs_dbgmsg("spa %llu arc flushed in %llu ms", - (u_longlong_t)guid, (u_longlong_t)elaspsed); + (u_longlong_t)spa_guid, (u_longlong_t)elaspsed); } } @@ -4438,15 +4484,44 @@ arc_flush_task(void *arg) void arc_flush_async(spa_t *spa) { - uint64_t *guidp = kmem_alloc(sizeof (uint64_t *), KM_SLEEP); + uint64_t spa_guid = spa_load_guid(spa); + arc_async_flush_t *af = arc_async_flush_add(spa_guid, TASKQID_INVALID); - *guidp = spa_load_guid(spa); + /* + * Note that arc_flush_task() needs arc_async_flush_lock to remove af + * list node. So by holding the lock we avoid a race for af removal + * with our use here. + */ + mutex_enter(&arc_async_flush_lock); + taskqid_t tid = af->af_task_id = taskq_dispatch(arc_flush_taskq, + arc_flush_task, af, TQ_SLEEP); + mutex_exit(&arc_async_flush_lock); + + /* + * unlikely, but if we couldn't dispatch then use an inline flush + */ + if (tid == TASKQID_INVALID) { + arc_async_flush_remove(spa_guid, TASKQID_INVALID); + arc_flush_impl(spa_guid, B_FALSE); + } +} - if (taskq_dispatch(arc_flush_taskq, arc_flush_task, guidp, - TQ_SLEEP) == TASKQID_INVALID) { - arc_flush_impl(*guidp, B_FALSE); - kmem_free(guidp, sizeof (uint64_t *)); +/* + * Check if a guid is still in-use as part of an async teardown task + */ +boolean_t +arc_async_flush_guid_inuse(uint64_t spa_guid) +{ + mutex_enter(&arc_async_flush_lock); + for (arc_async_flush_t *af = list_head(&arc_async_flush_list); + af != NULL; af = list_next(&arc_async_flush_list, af)) { + if (af->af_spa_guid == spa_guid) { + mutex_exit(&arc_async_flush_lock); + return (B_TRUE); + } } + mutex_exit(&arc_async_flush_lock); + return (B_FALSE); } void @@ -7706,6 +7781,9 @@ arc_init(void) arc_prune_taskq = taskq_create("arc_prune", zfs_arc_prune_task_threads, defclsyspri, 100, INT_MAX, TASKQ_PREPOPULATE | TASKQ_DYNAMIC); + list_create(&arc_async_flush_list, sizeof (arc_async_flush_t), + offsetof(arc_async_flush_t, af_node)); + mutex_init(&arc_async_flush_lock, NULL, MUTEX_DEFAULT, NULL); arc_flush_taskq = taskq_create("arc_flush", 75, defclsyspri, 1, INT_MAX, TASKQ_DYNAMIC | TASKQ_THREADS_CPU_PCT); @@ -7789,6 +7867,9 @@ arc_fini(void) taskq_wait(arc_prune_taskq); taskq_destroy(arc_prune_taskq); + list_destroy(&arc_async_flush_list); + mutex_destroy(&arc_async_flush_lock); + mutex_enter(&arc_prune_mtx); while ((p = list_remove_head(&arc_prune_list)) != NULL) { (void) zfs_refcount_remove(&p->p_refcnt, &arc_prune_list); @@ -9700,6 +9781,7 @@ typedef struct { l2arc_dev_t *rva_l2arc_dev; uint64_t rva_spa_gid; uint64_t rva_vdev_gid; + taskqid_t rva_task_id; } remove_vdev_args_t; static void @@ -9730,6 +9812,10 @@ l2arc_device_teardown(void *arg) (u_longlong_t)rva->rva_vdev_gid, (u_longlong_t)elaspsed); } + + if (rva->rva_task_id != TASKQID_INVALID) + arc_async_flush_remove(rva->rva_spa_gid, rva->rva_task_id); + kmem_free(rva, sizeof (remove_vdev_args_t)); } @@ -9788,11 +9874,22 @@ l2arc_remove_vdev(vdev_t *vd) } mutex_exit(&l2arc_dev_mtx); - /* - * If possible, the teardown is completed asynchronously - */ - if (!asynchronous || taskq_dispatch(arc_flush_taskq, - l2arc_device_teardown, rva, TQ_SLEEP) == TASKQID_INVALID) { + if (!asynchronous) { + l2arc_device_teardown(rva); + return; + } + + uint64_t spa_guid = spa_load_guid(spa); + arc_async_flush_t *af = arc_async_flush_add(spa_guid, TASKQID_INVALID); + + mutex_enter(&arc_async_flush_lock); + taskqid_t tid = taskq_dispatch(arc_flush_taskq, l2arc_device_teardown, + rva, TQ_SLEEP); + rva->rva_task_id = af->af_task_id = tid; + mutex_exit(&arc_async_flush_lock); + + if (tid == TASKQID_INVALID) { + arc_async_flush_remove(spa_guid, TASKQID_INVALID); l2arc_device_teardown(rva); } } diff --git a/module/zfs/spa_misc.c b/module/zfs/spa_misc.c index d1d41bbe7214..376f0e850174 100644 --- a/module/zfs/spa_misc.c +++ b/module/zfs/spa_misc.c @@ -1588,6 +1588,34 @@ spa_generate_guid(spa_t *spa) return (guid); } +static boolean_t +spa_load_guid_exists(uint64_t guid) +{ + avl_tree_t *t = &spa_namespace_avl; + + ASSERT(MUTEX_HELD(&spa_namespace_lock)); + + for (spa_t *spa = avl_first(t); spa != NULL; spa = AVL_NEXT(t, spa)) { + if (spa_load_guid(spa) == guid) + return (B_TRUE); + } + + return (arc_async_flush_guid_inuse(guid)); +} + +uint64_t +spa_generate_load_guid(void) +{ + uint64_t guid; + + do { + (void) random_get_pseudo_bytes((void *)&guid, + sizeof (guid)); + } while (guid == 0 || spa_load_guid_exists(guid)); + + return (guid); +} + void snprintf_blkptr(char *buf, size_t buflen, const blkptr_t *bp) { diff --git a/module/zfs/vdev.c b/module/zfs/vdev.c index 414bf84f6f7a..37e8708fe5fd 100644 --- a/module/zfs/vdev.c +++ b/module/zfs/vdev.c @@ -626,7 +626,7 @@ vdev_alloc_common(spa_t *spa, uint_t id, uint64_t guid, vdev_ops_t *ops) if (spa->spa_root_vdev == NULL) { ASSERT(ops == &vdev_root_ops); spa->spa_root_vdev = vd; - spa->spa_load_guid = spa_generate_guid(NULL); + spa->spa_load_guid = spa_generate_load_guid(); } if (guid == 0 && ops != &vdev_hole_ops) {