Reuse VG
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
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
@@ -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
@@ -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
@@ -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
@@ -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
PATCH 2/3 Pool locking code
In reply to Zdenek KabelacAdding 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
@@ -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
@@ -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
@@ -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
PATCH 3/3 Use dm_pool locking
In reply to Zdenek KabelacAdd 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
@@ -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
@@ -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