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

@@ -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);
 

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 */

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.
 	 */

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 dm_list *dm_devs; /* dm_active_device structs with dm UUIDs from DM_DEVICE_LIST (when available) */
-	struct radix_tree *dm_devnos;
+	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;
+	return _cache.use_dm_uuid_cache;
-	unsigned devs_features;
+}
-	uint32_t d;
 
-	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);
 

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 *

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(lvl, lvs)
-		dm_list_iterate_items(dm_dev, cmd->cache_dm_devs)
+		label_scan_invalidate_lv(cmd, lvl->lv);
-			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);
 }
 
 /*

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;