Skip to contentSkip to conversation

Reuse VG

PATCH New Zdenek Kabelac 1 participant 4 messages Jul 8, 2011

lore
Share & tools
lore
#0 lore raw

cover Reuse VG

Reuse VG structure multiple times.
With separate debug patch.

Zdenek Kabelac (3):
Share VG multiple times
Pool locking code
Use dm_pool locking

 lib/cache/lvmcache.c    |   74 ++++++++++++++++++++++++++++++++-
 lib/cache/lvmcache.h    |    5 ++
 lib/metadata/metadata.c |   42 ++++++++++++++++++-
 lib/metadata/vg.h       |    2 +
 libdm/libdevmapper.h    |    7 +++
 libdm/mm/pool-debug.c   |   27 ++++++++++++
 libdm/mm/pool-fast.c    |   66 ++++++++++++++++++++++++++++-
 libdm/mm/pool.c         |  106 ++++++++++++++++++++++++++++++++++++++++++++++-
 make.tmpl.in            |    2 +
 9 files changed, 325 insertions(+), 6 deletions(-)
Signature · 1 line
1.7.6
#1 lore raw

PATCH 1/3 Share VG multiple times

Saves CPU by skipping creation of the same volume_group structure.

Structure vginfo is extended vg_cache and counters.

It tracks the number of reuses of every cached VG.
For lv_suspend we occasionaly need to reference the same VG more then once - it's the reason for reference counter.

Caching is not used when VG is opened RW recover mode.

As there is far more case when sharing is possible - use rather specific code to drop VG from lvmcache in this case - such VG is then unlocked. and it is fully modifiable.

---
 lib/cache/lvmcache.c    |   67 ++++++++++++++++++++++++++++++++++++++++++++++-
 lib/cache/lvmcache.h    |    5 +++
 lib/metadata/metadata.c |   26 +++++++++++++++++-
 lib/metadata/vg.h       |    2 +
 4 files changed, 97 insertions(+), 3 deletions(-)
lib/cache/lvmcache.c +66 -1
index 0e92fa3..275087f 100644
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2001-2004 Sistina Software, Inc. All rights reserved.
- * Copyright (C) 2004-2008 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2004-2011 Red Hat, Inc. All rights reserved.
  *
  * This file is part of LVM2.
  *
@@ -89,6 +89,12 @@ static void _free_cached_vgmetadata(struct lvmcache_vginfo *vginfo)
 		vginfo->cft = NULL;
 	}
 
+	/* Release cached VG, and check there is at most 1 user */
+	/* Note: 1 reference is allowed during precommit lvmcache update */
+	free_vg(vginfo->vg_cache);
+	if (vginfo->vg_cache && !lvmcache_release_vg(vginfo->vg_cache))
+		log_error(INTERNAL_ERROR "Cache releases referenced VG.");
+
 	log_debug("Metadata cache: VG %s wiped.", vginfo->vgname);
 }
 
@@ -662,6 +668,17 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 	    (!precommitted && vginfo->precommitted && !critical_section()))
 		return NULL;
 
+	/* Use already cached VG when available */
+	if (vginfo->vg_cache)  {
+		if (vginfo->vg_ref_count > 2)
+			/* For lv_suspend we need lv and lv_precommit */
+			log_error(INTERNAL_ERROR "More then one shared VG.");
+		vg = vginfo->vg_cache;
+		vginfo->vg_ref_count++;
+		vginfo->vg_use_count++;
+		goto out;
+	}
+
 	fic.type = FMT_INSTANCE_VG | FMT_INSTANCE_MDAS | FMT_INSTANCE_AUX_MDAS;
 	fic.context.vg_ref.vg_name = vginfo->vgname;
 	fic.context.vg_ref.vg_id = vgid;
@@ -678,6 +695,13 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 	if (!(vg = import_vg_from_config_tree(vginfo->cft, fid)))
 		goto_bad;
 
+	/* Put VG into lvmcache */
+	vg->vginfo = vginfo;
+	vginfo->vg_cache = vg;
+	vginfo->vg_ref_count = 2; /* Keep 1 reference for lvmcache */
+	vginfo->vg_use_count = 1;
+
+out:
 	log_debug("Using cached %smetadata for VG %s.",
 		  vginfo->precommitted ? "pre-committed" : "", vginfo->vgname);
 
@@ -689,6 +713,47 @@ bad:
 	return NULL;
 }
 
+/**
+ * Release a referenced VG from lvmcache.
+ */
+int lvmcache_release_vg(struct volume_group *vg)
+{
+	if (!vg->vginfo)
+		return 1;
+
+	if (--vg->vginfo->vg_ref_count)
+		return 0; /* Still referenced */
+
+	log_debug("Released VG:%p %s from cache (used:%d).",
+		  vg, vg->name, vg->vginfo->vg_use_count);
+
+	vg->vginfo->vg_cache = NULL;
+	vg->vginfo = NULL;
+
+	return 1;
+}
+
+/**
+ * Drop VG from lvmcache.
+ *
+ * This is used for 'write' operations.
+ * Note: VG must not be shared in this moment.
+ */
+int lvmcache_drop_vg(struct volume_group *vg)
+{
+	if (!vg->vginfo)
+		return 1; /* Not cached */
+
+	free_vg(vg);
+
+	if (!lvmcache_release_vg(vg)) {
+		log_error(INTERNAL_ERROR "Dropping referenced VG.");
+		return 0;
+	}
+
+	return 1;
+}
+
 struct dm_list *lvmcache_get_vgids(struct cmd_context *cmd,
 				   int include_internal)
 {
lib/cache/lvmcache.h +5 -0
index 9aafff5..f5c1131 100644
@@ -50,6 +50,9 @@ struct lvmcache_vginfo {
 	char *vgmetadata;	/* Copy of VG metadata as format_text string */
 	struct config_tree *cft; /* Config tree created from vgmetadata */
 				/* Lifetime is directly tied to vgmetadata */
+	struct volume_group *vg_cache; /* Cached VG */
+	unsigned vg_ref_count; /* Cached VG ref counter */
+	unsigned vg_use_count; /* Cached VG usage */
 	unsigned precommitted;	/* Is vgmetadata live or precommitted? */
 };
 
@@ -122,6 +125,8 @@ struct dm_list *lvmcache_get_pvids(struct cmd_context *cmd, const char *vgname,
 
 /* Returns cached volume group metadata. */
 struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted);
+int lvmcache_release_vg(struct volume_group *vg);
+int lvmcache_drop_vg(struct volume_group *vg);
 void lvmcache_drop_metadata(const char *vgname, int drop_precommitted);
 void lvmcache_commit_metadata(const char *vgname);
 
lib/metadata/metadata.c +24 -2
index 842885f..d7fb6c3 100644
@@ -866,6 +866,12 @@ static struct volume_group *_vg_make_handle(struct cmd_context *cmd,
 					    struct volume_group *vg,
 					    uint32_t failure)
 {
+	if (vg && vg->vginfo && (failure != SUCCESS)) {
+		/* Avoid overwrite of cached VG */
+		free_vg(vg);
+		vg = NULL;
+	}
+
 	if (!vg && !(vg = alloc_vg("vg_make_handle", cmd, NULL)))
 		return_NULL;
 
@@ -2865,8 +2871,13 @@ static struct volume_group *_vg_read(struct cmd_context *cmd,
 	    (use_precommitted || !*consistent || !(correct_vg->status & INCONSISTENT_VG))) {
 		if (!(correct_vg->status & INCONSISTENT_VG))
 			*consistent = 1;
-		else	/* Inconsistent but we can't repair it */
+		else {
+			/* Inconsistent VG is being modified, no caching */
+			if (!lvmcache_drop_vg(correct_vg))
+				return_NULL;
+			/* Inconsistent but we can't repair it */
 			correct_vg->status &= ~INCONSISTENT_VG;
+		}
 
 		return correct_vg;
 	} else {
@@ -3312,6 +3323,11 @@ void free_vg(struct volume_group *vg)
 	if (!vg)
 		return;
 
+	log_debug("Releasing VG %s (vg:%p, info:%p).", vg->name, vg, vg->vginfo);
+
+	if (!lvmcache_release_vg(vg))
+		return;    /* Still referenced, do not destroy mempool */
+
 	vg_set_fid(vg, NULL);
 
 	if (vg->cmd && vg->vgmem == vg->cmd->mem) {
@@ -3850,7 +3866,13 @@ static struct volume_group *_recover_vg(struct cmd_context *cmd,
 		return_NULL;
 	}
 
-	return (struct volume_group *)vg;
+	/* Test if the VG is not accidentally from lvmcache */
+	if (vg->vginfo) {
+		log_error(INTERNAL_ERROR "Recovered VG is in the cache.");
+		return NULL;
+	}
+
+	return vg;
 }
 
 /*
lib/metadata/vg.h +2 -0
index bebe6cf..2cda69b 100644
@@ -20,6 +20,7 @@ struct dm_pool;
 struct format_instance;
 struct dm_list;
 struct id;
+struct lvmcache_vginfo;
 
 typedef enum {
 	ALLOC_INVALID,
@@ -41,6 +42,7 @@ struct volume_group {
 	struct cmd_context *cmd;
 	struct dm_pool *vgmem;
 	struct format_instance *fid;
+	struct lvmcache_vginfo *vginfo;	/* Backward reference to lvmcache */
 	struct dm_list *cmd_vgs;/* List of wanted/locked and opened VGs */
 	uint32_t cmd_missing_vgs;/* Flag marks missing VG */
 	uint32_t seqno;		/* Metadata sequence number */
Signature · 1 line
1.7.6
#2 lore raw

PATCH 2/3 Pool locking code

In reply to Zdenek Kabelac

Adding specific debug functions to lock and unlock memory pool.

2 ways to debug code:
mprotect - quite fast all the time - but requires more memory and
currently it is using posix_memalign() - this could be later modified to use dm_malloc() and align internally. Tool segfaults when locked memory is modified and core could be checked for problematic code section.

crc - checksum of locked pool - implemented via quick simple hash.
But this may get quite slow when the pool is larger - so the check is only made when VG is finaly released and it has been used more then once - so the result it's rather informative - and mprotect recompiled version is needed to detect real cause of troubles.

Only fast pools are using mprotect - so such build cannot be combined with DEBUG_POOL.

---
 libdm/libdevmapper.h  |    7 +++
 libdm/mm/pool-debug.c |   27 ++++++++++++
 libdm/mm/pool-fast.c  |   66 +++++++++++++++++++++++++++++-
 libdm/mm/pool.c       |  106 ++++++++++++++++++++++++++++++++++++++++++++++++-
 make.tmpl.in          |    2 +
 5 files changed, 205 insertions(+), 3 deletions(-)
libdm/libdevmapper.h +7 -0
index 340e6e5..6521032 100644
@@ -605,6 +605,13 @@ void *dm_pool_alloc_aligned(struct dm_pool *p, size_t s, unsigned alignment);
 void dm_pool_empty(struct dm_pool *p);
 void dm_pool_free(struct dm_pool *p, void *ptr);
 
+/* pool locking routines */
+int dm_pool_locked(struct dm_pool *p);
+int dm_pool_lock(struct dm_pool *p, int crc)
+	__attribute__((__warn_unused_result__));
+int dm_pool_unlock(struct dm_pool *p, int crc)
+	__attribute__((__warn_unused_result__));
+
 /*
  * Object building routines:
  *
libdm/mm/pool-debug.c +27 -0
index af1a912..d8c19d4 100644
@@ -33,6 +33,8 @@ struct dm_pool {
 	struct dm_list list;
 	const char *name;
 	void *orig_pool;	/* to pair it with first allocation call */
+	unsigned locked;
+	long crc;
 
 	int begun;
 	struct block *object;
@@ -71,6 +73,8 @@ static void _free_blocks(struct dm_pool *p, struct block *b)
 {
 	struct block *n;
 
+	assert(!p->locked);
+
 	while (b) {
 		p->stats.bytes -= b->size;
 		p->stats.blocks_allocated--;
@@ -84,6 +88,8 @@ static void _free_blocks(struct dm_pool *p, struct block *b)
 
 static void _pool_stats(struct dm_pool *p, const char *action)
 {
+	assert(!p->locked);
+
 #ifdef DEBUG_POOL
 	log_debug("%s mempool %s: %u/%u bytes, %u/%u blocks, "
 		  "%u allocations)", action, p->name, p->stats.bytes,
@@ -99,6 +105,7 @@ void dm_pool_destroy(struct dm_pool *p)
 	_pool_stats(p, "Destroying");
 	_free_blocks(p, p->blocks);
 	dm_list_del(&p->list);
+	dm_free(p->volatile_mem);
 	dm_free(p);
 }
 
@@ -109,6 +116,8 @@ void *dm_pool_alloc(struct dm_pool *p, size_t s)
 
 static void _append_block(struct dm_pool *p, struct block *b)
 {
+	assert(!p->locked);
+
 	if (p->tail) {
 		p->tail->next = b;
 		p->tail = b;
@@ -216,6 +225,8 @@ int dm_pool_grow_object(struct dm_pool *p, const void *extra, size_t delta)
 	struct block *new;
 	size_t new_size;
 
+	assert(!p->locked);
+
 	if (!delta)
 		delta = strlen(extra);
 
@@ -260,3 +271,19 @@ void dm_pool_abandon_object(struct dm_pool *p)
 	p->begun = 0;
 	p->object = NULL;
 }
+
+static long _pool_crc(const struct dm_pool *p)
+{
+#ifndef DEBUG_USE_MPROTECT
+#warning pool crc not implemented with pool debug
+#endif
+	return 0;
+}
+
+static int _pool_protect(struct dm_pool *p, int prot)
+{
+#ifdef DEBUG_USE_MPROTECT
+#warning pool mprotect not implemented with pool debug
+#endif
+	return 1;
+}
libdm/mm/pool-fast.c +64 -2

92 lines not on this page · raw message

libdm/mm/pool.c +105 -1

125 lines not on this page · raw message

make.tmpl.in +2 -0
index cd8ae35..d52f7ca 100644
@@ -149,6 +149,8 @@ ifeq ("@DM_IOCTLS@", "yes")
 endif
 
 #DEFS += -DDEBUG_POOL
+# do not use DEBUG_POOL and DEBUG_USE_MPROTECT at the same time
+#DEFS += -DDEBUG_USE_MPROTECT
 #DEFS += -DBOUNDS_CHECK
 
 #CFLAGS += -pg
Signature · 1 line
1.7.6
#3 lore raw

PATCH 3/3 Use dm_pool locking

In reply to Zdenek Kabelac

Add debug pool locking functionality. So the command could check, whether the memory in the pool was not modified.

For lv_postoder() instead of unlocking and locking for every changed struct status member do it once when entering and leaving function. Currently lv_postoder() does not modify other part of vg structure then status flags of each LV with flags that are reverted back to its original state after fucntion exit.

---
 lib/cache/lvmcache.c    |    7 +++++++
 lib/metadata/metadata.c |   16 ++++++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)
lib/cache/lvmcache.c +7 -0
index 275087f..19bae10 100644
@@ -701,6 +701,9 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 	vginfo->vg_ref_count = 2; /* Keep 1 reference for lvmcache */
 	vginfo->vg_use_count = 1;
 
+	if (!dm_pool_lock(vg->vgmem, 1))
+		goto_bad;
+
 out:
 	log_debug("Using cached %smetadata for VG %s.",
 		  vginfo->precommitted ? "pre-committed" : "", vginfo->vgname);
@@ -727,6 +730,10 @@ int lvmcache_release_vg(struct volume_group *vg)
 	log_debug("Released VG:%p %s from cache (used:%d).",
 		  vg, vg->name, vg->vginfo->vg_use_count);
 
+	/* Debug perform crc check only when it's been used more then once */
+	if (!dm_pool_unlock(vg->vgmem, vg->vginfo->vg_use_count > 1))
+		log_error(INTERNAL_ERROR "VG mempool.");
+
 	vg->vginfo->vg_cache = NULL;
 	vg->vginfo = NULL;
 
lib/metadata/metadata.c +16 -0
index d7fb6c3..da15fc9 100644
@@ -2107,8 +2107,17 @@ static int _lv_postorder(struct logical_volume *lv,
 			       void *data)
 {
 	int r;
+	int lck = dm_pool_locked(lv->vg->vgmem);
+
+	if (lck && !dm_pool_unlock(lv->vg->vgmem, 0))
+		return_0;
+
 	r = _lv_postorder_visit(lv, fn, data);
 	_lv_postorder_cleanup(lv, 0);
+
+	if (lck && !dm_pool_lock(lv->vg->vgmem, 0))
+		return_0;
+
 	return r;
 }
 
@@ -2122,6 +2131,10 @@ static int _lv_postorder_vg(struct volume_group *vg,
 {
 	struct lv_list *lvl;
 	int r = 1;
+	int lck = dm_pool_locked(vg->vgmem);
+
+	if (lck && !dm_pool_unlock(vg->vgmem, 0))
+		return_0;
 
 	dm_list_iterate_items(lvl, &vg->lvs)
 		if (!_lv_postorder_visit(lvl->lv, fn, data)) {
@@ -2132,6 +2145,9 @@ static int _lv_postorder_vg(struct volume_group *vg,
 	dm_list_iterate_items(lvl, &vg->lvs)
 		_lv_postorder_cleanup(lvl->lv, 0);
 
+	if (lck && !dm_pool_lock(vg->vgmem, 0))
+		return_0;
+
 	return r;
 }
 
Signature · 1 line
1.7.6