From b8869e2d4e10f0fa7bf69fd1fb97b4a59c469db7 Mon Sep 17 00:00:00 2001 From: David Teigland Date: Fri, 28 Jun 2024 17:57:00 -0500 Subject: [PATCH] dev-cache: unify dm uuid cache struct lifetimes The list of dm devs was in the cmd struct and had a different lifetime than the radix trees referencing those dm devs. Now the list and radix trees are created and destroyed together. --- lib/activate/dev_manager.c | 12 +++++----- lib/commands/toolcontext.c | 2 -- lib/commands/toolcontext.h | 2 -- lib/device/dev-cache.c | 48 +++++++++++++++++++++++++------------- lib/device/dev-cache.h | 4 +++- lib/label/label.c | 21 +++++------------ lib/locking/locking.c | 2 +- 7 files changed, 48 insertions(+), 43 deletions(-) diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index c7f0ec086..6651827e4 100644 --- a/lib/activate/dev_manager.c +++ b/lib/activate/dev_manager.c @@ -766,7 +766,7 @@ int dm_device_is_usable(struct cmd_context *cmd, struct device *dev, struct dev_ int only_error_or_zero_target = 1; int r = 0; - if (cmd->cache_dm_devs && + if (dev_cache_use_dm_uuid_cache() && /* With cache we can avoid status calls for unusable UUIDs */ (dm_dev = dev_cache_get_dm_dev_by_devno(cmd, dev->dev)) && !_is_usable_uuid(dev, dm_dev->name, dm_dev->uuid, check.check_reserved, check.check_lv, is_lv)) @@ -897,7 +897,7 @@ int devno_dm_uuid(struct cmd_context *cmd, int major, int minor, const char *uuid; int r = 0; - if (cmd->cache_dm_devs) { + if (dev_cache_use_dm_uuid_cache()) { if ((dm_dev = dev_cache_get_dm_dev_by_devno(cmd, MKDEV(major, minor)))) { dm_strncpy(uuid_buf, dm_dev->uuid, uuid_buf_size); return 1; @@ -1085,7 +1085,7 @@ int dev_manager_info(struct cmd_context *cmd, dm_strncpy(old_style_dlid, dlid, sizeof(old_style_dlid)); - if (cmd->cache_dm_devs && + if (dev_cache_use_dm_uuid_cache() && !dev_cache_get_dm_dev_by_uuid(cmd, dlid) && !dev_cache_get_dm_dev_by_uuid(cmd, old_style_dlid)) { log_debug("Cached as inactive %s.", name); @@ -2459,7 +2459,7 @@ static int _add_dev_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, if (!(dlid = build_dm_uuid(dm->track_pending_delete ? dm->cmd->pending_delete_mem : dm->mem, lv, layer))) return_0; - if (dm->cmd->cache_dm_devs) { + if (dev_cache_use_dm_uuid_cache()) { if (!(dm_dev = dev_cache_get_dm_dev_by_uuid(dm->cmd, dlid))) { log_debug("Cached as not present %s.", name); return 1; @@ -2614,7 +2614,7 @@ static int _pool_callback(struct dm_tree_node *node, } } - dm_device_list_destroy(&cmd->cache_dm_devs); /* Cache no longer valid */ + dev_cache_destroy_dm_uuids(); log_debug("Running check command on %s", mpath); @@ -3998,7 +3998,7 @@ static int _tree_action(struct dev_manager *dm, const struct logical_volume *lv, /* Drop any cache before DM table manipulation within locked section * TODO: check if it makes sense to manage cache within lock */ - dm_device_list_destroy(&dm->cmd->cache_dm_devs); + dev_cache_destroy_dm_uuids(); dtree = _create_partial_dtree(dm, lv, laopts->origin_only); diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c index 2ad04d7ec..56dc1f856 100644 --- a/lib/commands/toolcontext.c +++ b/lib/commands/toolcontext.c @@ -2038,8 +2038,6 @@ void destroy_toolcontext(struct cmd_context *cmd) if (cmd->cft_def_hash) dm_hash_destroy(cmd->cft_def_hash); - dm_device_list_destroy(&cmd->cache_dm_devs); - if (!cmd->running_on_valgrind && cmd->linebuffer) { int flags; /* Reset stream buffering to defaults */ diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h index 0f93813d7..b405bbf00 100644 --- a/lib/commands/toolcontext.h +++ b/lib/commands/toolcontext.h @@ -230,8 +230,6 @@ struct cmd_context { const char *devicesfile; /* from --devicesfile option */ struct dm_list deviceslist; /* from --devices option, struct dm_str_list */ - struct dm_list *cache_dm_devs; /* cache with UUIDs from DM_DEVICE_LIST (when available) */ - /* * Configuration. */ diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c index 217a45e71..8dc27697b 100644 --- a/lib/device/dev-cache.c +++ b/lib/device/dev-cache.c @@ -52,12 +52,14 @@ static struct { struct radix_tree *names; struct dm_hash_table *vgid_index; struct dm_hash_table *lvid_index; - struct radix_tree *dm_uuids; - struct radix_tree *dm_devnos; + struct dm_list *dm_devs; /* dm_active_device structs with dm UUIDs from DM_DEVICE_LIST (when available) */ + struct radix_tree *dm_uuids; /* references dm_devs entries */ + struct radix_tree *dm_devnos; /* references dm_devs entries */ struct radix_tree *sysfs_only_devices; /* see comments in _get_device_for_sysfs_dev_name_using_devno */ struct radix_tree *devices; struct dm_regex *preferred_names_matcher; const char *dev_dir; + int use_dm_uuid_cache; size_t dev_dir_len; int has_scanned; @@ -1325,13 +1327,14 @@ out: return r; } -int dev_cache_update_dm_devs(struct cmd_context *cmd) +int dev_cache_use_dm_uuid_cache(void) { - struct dm_active_device *dm_dev; - unsigned devs_features; - uint32_t d; + return _cache.use_dm_uuid_cache; +} - dm_device_list_destroy(&cmd->cache_dm_devs); +void dev_cache_destroy_dm_uuids(void) +{ + _cache.use_dm_uuid_cache = 0; if (_cache.dm_devnos) { radix_tree_destroy(_cache.dm_devnos); @@ -1343,22 +1346,38 @@ int dev_cache_update_dm_devs(struct cmd_context *cmd) _cache.dm_uuids = NULL; } - if (!get_dm_active_devices(NULL, &cmd->cache_dm_devs, &devs_features)) + dm_device_list_destroy(&_cache.dm_devs); +} + +int dev_cache_update_dm_uuids(void) +{ + struct dm_active_device *dm_dev; + unsigned devs_features; + uint32_t d; + + dev_cache_destroy_dm_uuids(); + + if (!get_dm_active_devices(NULL, &_cache.dm_devs, &devs_features)) return 1; if (!(devs_features & DM_DEVICE_LIST_HAS_UUID)) { /* Cache unusable with older kernels without UUIDs in LIST */ - dm_device_list_destroy(&cmd->cache_dm_devs); + dm_device_list_destroy(&_cache.dm_devs); return 1; } + /* _cache.dm_devs entries are referenced by radix trees */ + + /* TODO: if _cache.dm_devs list is small, then skip the + overhead of radix trees and just do list searches on dm_devs */ + if (!(_cache.dm_devnos = radix_tree_create(NULL, NULL)) || !(_cache.dm_uuids = radix_tree_create(NULL, NULL))) { return_0; // FIXME } /* Insert every active DM device into radix trees */ - dm_list_iterate_items(dm_dev, cmd->cache_dm_devs) { + dm_list_iterate_items(dm_dev, _cache.dm_devs) { d = _shuffle_devno(dm_dev->devno); if (!radix_tree_insert_ptr(_cache.dm_devnos, &d, sizeof(d), dm_dev)) @@ -1372,6 +1391,7 @@ int dev_cache_update_dm_devs(struct cmd_context *cmd) //radix_tree_dump(_cache.dm_devnos, stdout); //radix_tree_dump(_cache.dm_uuids, stdout); + _cache.use_dm_uuid_cache = 1; return 1; } @@ -1503,6 +1523,8 @@ int dev_cache_exit(void) vt.num_open); } + dev_cache_destroy_dm_uuids(); + if (_cache.mem) dm_pool_destroy(_cache.mem); @@ -1515,12 +1537,6 @@ int dev_cache_exit(void) if (_cache.lvid_index) dm_hash_destroy(_cache.lvid_index); - if (_cache.dm_devnos) - radix_tree_destroy(_cache.dm_devnos); - - if (_cache.dm_uuids) - radix_tree_destroy(_cache.dm_uuids); - if (_cache.devices) radix_tree_destroy(_cache.devices); diff --git a/lib/device/dev-cache.h b/lib/device/dev-cache.h index 6cb1bf728..0f56ba702 100644 --- a/lib/device/dev-cache.h +++ b/lib/device/dev-cache.h @@ -37,7 +37,9 @@ struct dev_filter { struct dm_list *dev_cache_get_dev_list_for_vgid(const char *vgid); struct dm_list *dev_cache_get_dev_list_for_lvid(const char *lvid); -int dev_cache_update_dm_devs(struct cmd_context *cmd); +int dev_cache_use_dm_uuid_cache(void); +int dev_cache_update_dm_uuids(void); +void dev_cache_destroy_dm_uuids(void); const struct dm_active_device * dev_cache_get_dm_dev_by_devno(struct cmd_context *cmd, dev_t devno); const struct dm_active_device * diff --git a/lib/label/label.c b/lib/label/label.c index d1e175cad..6fda88dc7 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -1263,7 +1263,7 @@ int label_scan(struct cmd_context *cmd) * here, before processing the hints file, so that the dm uuid checks * in hint processing can benefit from the dm uuid cache.) */ - if (!dev_cache_update_dm_devs(cmd)) + if (!dev_cache_update_dm_uuids()) return_0; /* @@ -1646,6 +1646,9 @@ void label_scan_invalidate_lv(struct cmd_context *cmd, struct logical_volume *lv struct device *dev; dev_t devt; + /* FIXME: use dev_cache_get_existing() with the lv name, + which allow us to skip the getting devno from lv_info. */ + if (lv_info(cmd, lv, 0, &lvinfo, 0, 0) && lvinfo.exists) { /* FIXME: Still unclear what is it supposed to find */ devt = MKDEV(lvinfo.major, lvinfo.minor); @@ -1656,8 +1659,6 @@ void label_scan_invalidate_lv(struct cmd_context *cmd, struct logical_volume *lv void label_scan_invalidate_lvs(struct cmd_context *cmd, struct dm_list *lvs) { - struct dm_active_device *dm_dev; - struct device *dev; struct lv_list *lvl; /* @@ -1669,18 +1670,8 @@ void label_scan_invalidate_lvs(struct cmd_context *cmd, struct dm_list *lvs) log_debug("Invalidating devs for any PVs on LVs."); - if (cmd->cache_dm_devs) { - dm_list_iterate_items(dm_dev, cmd->cache_dm_devs) - if (dm_dev->uuid && - strncmp(dm_dev->uuid, UUID_PREFIX, sizeof(UUID_PREFIX) - 1) == 0) { - if ((dev = dev_cache_get_by_devt(cmd, dm_dev->devno))) - label_scan_invalidate(dev); - } - } else - /* With older kernels without UUIDs we have to go the old way - * and check for every LVs UUID one by one */ - dm_list_iterate_items(lvl, lvs) - label_scan_invalidate_lv(cmd, lvl->lv); + dm_list_iterate_items(lvl, lvs) + label_scan_invalidate_lv(cmd, lvl->lv); } /* diff --git a/lib/locking/locking.c b/lib/locking/locking.c index 8ce979826..3a38c4109 100644 --- a/lib/locking/locking.c +++ b/lib/locking/locking.c @@ -330,7 +330,7 @@ int vg_write_lock_held(void) int sync_local_dev_names(struct cmd_context* cmd) { - dm_device_list_destroy(&cmd->cache_dm_devs); + dev_cache_destroy_dm_uuids(); memlock_unlock(cmd); fs_unlock(); return 1;