From f73be4480a5dd104a77e3ef84d7dcc80b834e593 Mon Sep 17 00:00:00 2001

From: David Teigland <teigland@redhat.com>

Date: Tue, 2 Nov 2021 15:42:26 -0500

Subject: [PATCH 06/23] filter-sysfs: skip when device id is set

When a device id is set for a device, using an idtype other

than devname, it means that sysfs has been used with the device

to match the device id.  So, checking for a sysfs entry for the

device in filter-sysfs is redundant.  For any other cases not

covered by this (e.g. devname ids), have filter-sysfs simply

stat /sys/dev/block/major:minor to test if the device exists

in sysfs.

The extensive processing done by filter-sysfs init is removed.

It was taking an immense amount of time with many devices, e.g.

. 1024 PVs in 520 VGs

. 520 concurrent vgchange -ay <vgname> commands

. vgchange scans only PVs in the named VG (based on pvs_online

  files from a pending patch)

A large number of the vgchange commands were taking over 1 min,

and nearly half of that time was used by filter-sysfs init.

With this patch, the vgchange commands take about half the time.

---

 lib/commands/toolcontext.c |  24 ++-

 lib/filters/filter-sysfs.c | 296 +++----------------------------------

 2 files changed, 32 insertions(+), 288 deletions(-)

diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c

index 1b7170de1..a0c78ddd6 100644

--- a/lib/commands/toolcontext.c

+++ b/lib/commands/toolcontext.c

@@ -1143,19 +1143,6 @@ static struct dev_filter *_init_filter_chain(struct cmd_context *cmd)

 	 * Update MAX_FILTERS definition above when adding new filters.

 	 */

-	/*

-	 * sysfs filter. Only available on 2.6 kernels.  Non-critical.

-	 * Listed first because it's very efficient at eliminating

-	 * unavailable devices.

-	 *

-	 * TODO: I suspect that using the lvm_type and device_id

-	 * filters before this one may be more efficient.

-	 */

-	if (find_config_tree_bool(cmd, devices_sysfs_scan_CFG, NULL)) {

-		if ((filters[nr_filt] = sysfs_filter_create()))

-			nr_filt++;

-	}

-

 	/* internal filter used by command processing. */

 	if (!(filters[nr_filt] = internal_filter_create())) {

 		log_error("Failed to create internal device filter");

@@ -1195,6 +1182,17 @@ static struct dev_filter *_init_filter_chain(struct cmd_context *cmd)

 	}

 	nr_filt++;

+	/*

+	 * sysfs filter. Only available on 2.6 kernels.  Non-critical.

+	 * Eliminates unavailable devices.

+	 * TODO: this may be unnecessary now with device ids

+	 * (currently not used for devs match to device id using syfs)

+	 */

+	if (find_config_tree_bool(cmd, devices_sysfs_scan_CFG, NULL)) {

+		if ((filters[nr_filt] = sysfs_filter_create()))

+			nr_filt++;

+	}

+

 	/* usable device filter. Required. */

 	if (!(filters[nr_filt] = usable_filter_create(cmd, cmd->dev_types, FILTER_MODE_NO_LVMETAD))) {

 		log_error("Failed to create usabled device filter");

diff --git a/lib/filters/filter-sysfs.c b/lib/filters/filter-sysfs.c

index 32ac324dd..672211057 100644

--- a/lib/filters/filter-sysfs.c

+++ b/lib/filters/filter-sysfs.c

@@ -17,288 +17,49 @@

 #ifdef __linux__

-#include <sys/sysmacros.h>

-#include <dirent.h>

-

-static int _locate_sysfs_blocks(const char *sysfs_dir, char *path, size_t len,

-				unsigned *sysfs_depth)

+static int _accept_p(struct cmd_context *cmd, struct dev_filter *f, struct device *dev, const char *use_filter_name)

 {

+	char path[PATH_MAX];

+	const char *sysfs_dir;

 	struct stat info;

-	unsigned i;

-	static const struct dir_class {

-		const char path[32];

-		int depth;

-	} classes[] = {

-		/*

-		 * unified classification directory for all kernel subsystems

-		 *

-		 * /sys/subsystem/block/devices

-		 * |-- sda -> ../../../devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda

-		 * |-- sda1 -> ../../../devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda/sda1

-		 *  `-- sr0 -> ../../../devices/pci0000:00/0000:00:1f.2/host1/target1:0:0/1:0:0:0/block/sr0

-		 *

-		 */

-		{ "subsystem/block/devices", 0 },

-

-		/*

-		 * block subsystem as a class

-		 *

-		 * /sys/class/block

-		 * |-- sda -> ../../devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda

-		 * |-- sda1 -> ../../devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda/sda1

-		 *  `-- sr0 -> ../../devices/pci0000:00/0000:00:1f.2/host1/target1:0:0/1:0:0:0/block/sr0

-		 *

-		 */

-		{ "class/block", 0 },

-

-		/*

-		 * old block subsystem layout with nested directories

-		 *

-		 * /sys/block/

-		 * |-- sda

-		 * |   |-- capability

-		 * |   |-- dev

-		 * ...

-		 * |   |-- sda1

-		 * |   |   |-- dev

-		 * ...

-		 * |

-		 * `-- sr0

-		 *     |-- capability

-		 *     |-- dev

-		 * ...

-		 *

-		 */

-

-		{ "block", 1 }

-	};

-

-	for (i = 0; i < DM_ARRAY_SIZE(classes); ++i)

-		if ((dm_snprintf(path, len, "%s%s", sysfs_dir, classes[i].path) >= 0) &&

-		    (stat(path, &info) == 0)) {

-			*sysfs_depth = classes[i].depth;

-			return 1;

-		}

-

-	return 0;

-}

-

-/*----------------------------------------------------------------

- * We need to store a set of dev_t.

- *--------------------------------------------------------------*/

-struct entry {

-	struct entry *next;

-	dev_t dev;

-};

-

-#define SET_BUCKETS 64

-struct dev_set {

-	struct dm_pool *mem;

-	const char *sys_block;

-	unsigned sysfs_depth;

-	int initialised;

-	struct entry *slots[SET_BUCKETS];

-};

-

-static struct dev_set *_dev_set_create(struct dm_pool *mem,

-				       const char *sys_block,

-				       unsigned sysfs_depth)

-{

-	struct dev_set *ds;

-

-	if (!(ds = dm_pool_zalloc(mem, sizeof(*ds))))

-		return NULL;

-

-	ds->mem = mem;

-	if (!(ds->sys_block = dm_pool_strdup(mem, sys_block)))

-		return NULL;

-

-	ds->sysfs_depth = sysfs_depth;

-	ds->initialised = 0;

-

-	return ds;

-}

-

-static unsigned _hash_dev(dev_t dev)

-{

-	return (major(dev) ^ minor(dev)) & (SET_BUCKETS - 1);

-}

-/*

- * Doesn't check that the set already contains dev.

- */

-static int _set_insert(struct dev_set *ds, dev_t dev)

-{

-	struct entry *e;

-	unsigned h = _hash_dev(dev);

-

-	if (!(e = dm_pool_alloc(ds->mem, sizeof(*e))))

-		return 0;

-

-	e->next = ds->slots[h];

-	e->dev = dev;

-	ds->slots[h] = e;

-

-	return 1;

-}

+	dev->filtered_flags &= ~DEV_FILTERED_SYSFS;

-static int _set_lookup(struct dev_set *ds, dev_t dev)

-{

-	unsigned h = _hash_dev(dev);

-	struct entry *e;

+	/*

+	 * Any kind of device id other than devname has been set

+	 * using sysfs so we know that sysfs info exists for dev.

+	 */

+	if (dev->id && dev->id->idtype && (dev->id->idtype != DEV_ID_TYPE_DEVNAME))

+		return 1;

-	for (e = ds->slots[h]; e; e = e->next)

-		if (e->dev == dev)

+	sysfs_dir = dm_sysfs_dir();

+	if (sysfs_dir && *sysfs_dir) {

+		if (dm_snprintf(path, sizeof(path), "%sdev/block/%d:%d",

+				sysfs_dir, (int)MAJOR(dev->dev), (int)MINOR(dev->dev)) < 0) {

+			log_debug("failed to create sysfs path");

 			return 1;

-

-	return 0;

-}

-

-/*----------------------------------------------------------------

- * filter methods

- *--------------------------------------------------------------*/

-static int _parse_dev(const char *file, FILE *fp, dev_t *result)

-{

-	unsigned major, minor;

-	char buffer[64];

-

-	if (!fgets(buffer, sizeof(buffer), fp)) {

-		log_error("Empty sysfs device file: %s", file);

-		return 0;

-	}

-

-	if (sscanf(buffer, "%u:%u", &major, &minor) != 2) {

-		log_error("Incorrect format for sysfs device file: %s.", file);

-		return 0;

-	}

-

-	*result = makedev(major, minor);

-	return 1;

-}

-

-static int _read_dev(const char *file, dev_t *result)

-{

-	int r;

-	FILE *fp;

-

-	if (!(fp = fopen(file, "r"))) {

-		log_sys_error("fopen", file);

-		return 0;

-	}

-

-	r = _parse_dev(file, fp, result);

-

-	if (fclose(fp))

-		log_sys_error("fclose", file);

-

-	return r;

-}

-

-/*

- * Recurse through sysfs directories, inserting any devs found.

- */

-static int _read_devs(struct dev_set *ds, const char *dir, unsigned sysfs_depth)

-{

-	struct dirent *d;

-	DIR *dr;

-	struct stat info;

-	char path[PATH_MAX];

-	char file[PATH_MAX];

-	dev_t dev = { 0 };

-	int r = 1;

-

-	if (!(dr = opendir(dir))) {

-		log_sys_error("opendir", dir);

-		return 0;

-	}

-

-	while ((d = readdir(dr))) {

-		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))

-			continue;

-

-		if (dm_snprintf(path, sizeof(path), "%s/%s", dir,

-				 d->d_name) < 0) {

-			log_warn("WARNING: sysfs path name too long: %s in %s.",

-				 d->d_name, dir);

-			continue;

 		}

-		/* devices have a "dev" file */

-		if (dm_snprintf(file, sizeof(file), "%s/dev", path) < 0) {

-			log_warn("WARNING: sysfs path name too long: %s in %s.",

-				 d->d_name, dir);

-			continue;

-		}

-

-		if (!stat(file, &info)) {

-			/* recurse if we found a device and expect subdirs */

-			if (sysfs_depth)

-				_read_devs(ds, path, sysfs_depth - 1);

-

-			/* add the device we have found */

-			if (_read_dev(file, &dev))

-				_set_insert(ds, dev);

+		if (lstat(path, &info)) {

+			log_debug_devs("%s: Skipping (sysfs)", dev_name(dev));

+			dev->filtered_flags |= DEV_FILTERED_SYSFS;

+			return 0;

 		}

 	}

-	if (closedir(dr))

-		log_sys_debug("closedir", dir);

-

-	return r;

-}

-

-static int _init_devs(struct dev_set *ds)

-{

-	if (!_read_devs(ds, ds->sys_block, ds->sysfs_depth)) {

-		ds->initialised = -1;

-		return 0;

-	}

-

-	ds->initialised = 1;

-

-	return 1;

-}

-

-

-static int _accept_p(struct cmd_context *cmd, struct dev_filter *f, struct device *dev, const char *use_filter_name)

-{

-	struct dev_set *ds = (struct dev_set *) f->private;

-

-	dev->filtered_flags &= ~DEV_FILTERED_SYSFS;

-

-	if (!ds->initialised)

-		_init_devs(ds);

-

-	/* Pass through if initialisation failed */

-	if (ds->initialised != 1)

-		return 1;

-

-	if (!_set_lookup(ds, dev->dev)) {

-		log_debug_devs("%s: Skipping (sysfs)", dev_name(dev));

-		dev->filtered_flags |= DEV_FILTERED_SYSFS;

-		return 0;

-	}

-

 	return 1;

 }

 static void _destroy(struct dev_filter *f)

 {

-	struct dev_set *ds = (struct dev_set *) f->private;

-

 	if (f->use_count)

 		log_error(INTERNAL_ERROR "Destroying sysfs filter while in use %u times.", f->use_count);

-

-	dm_pool_destroy(ds->mem);

+	free(f);

 }

 struct dev_filter *sysfs_filter_create(void)

 {

 	const char *sysfs_dir = dm_sysfs_dir();

-	char sys_block[PATH_MAX];

-	unsigned sysfs_depth;

-	struct dm_pool *mem;

-	struct dev_set *ds;

 	struct dev_filter *f;

 	if (!*sysfs_dir) {

@@ -306,26 +67,12 @@ struct dev_filter *sysfs_filter_create(void)

 		return NULL;

 	}

-	if (!_locate_sysfs_blocks(sysfs_dir, sys_block, sizeof(sys_block), &sysfs_depth))

-		return NULL;

-

-	if (!(mem = dm_pool_create("sysfs", 256))) {

-		log_error("sysfs pool creation failed");

-		return NULL;

-	}

-

-	if (!(ds = _dev_set_create(mem, sys_block, sysfs_depth))) {

-		log_error("sysfs dev_set creation failed");

-		goto bad;

-	}

-

-	if (!(f = dm_pool_zalloc(mem, sizeof(*f))))

+	if (!(f = zalloc(sizeof(*f))))

 		goto_bad;

 	f->passes_filter = _accept_p;

 	f->destroy = _destroy;

 	f->use_count = 0;

-	f->private = ds;

 	f->name = "sysfs";

 	log_debug_devs("Sysfs filter initialised.");

@@ -333,7 +80,6 @@ struct dev_filter *sysfs_filter_create(void)

 	return f;

  bad:

-	dm_pool_destroy(mem);

 	return NULL;

 }

-- 

2.31.1