[PATCH 0/5] Fix some scaling problems for large VGs (many LVs)

[PATCH 0/5] Fix some scaling problems for large VGs (many LVs)

[Milan Broz]

This patches fixes some problems in code when processing large VGs.

An example: VG with one PVs and 2000LVs
(I was not able to test more LVs without patches, swapped to death...)

# vgs vg_test
  VG      #PV #LV  #SN Attr   VSize  VFree
  vg_test   1 2000   0 wz--n- 18.53g 10.71g

Before (intentionally do not run any in-kernel operations -t test mode):

# time vgchange -t -a n vg_test
  Test mode: Metadata will NOT be updated.
    0 logical volume(s) in volume group "vg_test" now active

    real    9m31.463s
    user    9m29.841s
    sys     0m1.251s

After (patches applied):
# time vgchange -t -a n vg_test
  Test mode: Metadata will NOT be updated.
   0 logical volume(s) in volume group "vg_test" now active

    real    1m22.852s
    user    1m14.805s
    sys     0m7.961s


Memory use (cca, it oscillates according to vg_release) 
before:
root     24664 99.9 17.9 355588 350340 pts/0   R+   14:43   9:34 vgchange -t -a n vg_test
after:
root     31022 98.5  0.2   7736   5084 pts/0   R+   14:57   1:21 vgchange -t -a n vg_test

...

Milan Broz (5):
  Use hash table for quick lv reference when reading metadata.
  Remove vg_validate call when parsing cached metadata.
  Optimise PV segments search.
  Do not traverse PV segment list twice.
  Fix all segments memory is allocated from vg private mempool.

 lib/cache/lvmcache.c          |    3 +-
 lib/format_text/import.c      |    2 +-
 lib/format_text/import_vsn1.c |   42 +++++++++++++++++++++-------
 lib/metadata/lv_manip.c       |    2 +-
 lib/metadata/merge.c          |    4 +-
 lib/metadata/metadata.c       |   12 --------
 lib/metadata/metadata.h       |    3 --
 lib/metadata/pv_alloc.h       |    4 ++-
 lib/metadata/pv_manip.c       |   60 ++++++++++++++++++++++++++++------------
 9 files changed, 81 insertions(+), 51 deletions(-)

[PATCH 1/5] Use hash table for quick lv reference when reading metadata.

[Milan Broz]

The _read_vg uses already hash for PVs to optimise
reading of large VGs and avoiding repeated PV list traversing.

Use the same aproach to speed up parsing VG with many LVs.

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 lib/format_text/import_vsn1.c |   42 ++++++++++++++++++++++++++++++----------
 1 files changed, 31 insertions(+), 11 deletions(-)

diff --git a/lib/format_text/import_vsn1.c b/lib/format_text/import_vsn1.c
index 7e7d60f..21928eb 100644
--- a/lib/format_text/import_vsn1.c
+++ b/lib/format_text/import_vsn1.c
@@ -28,6 +28,7 @@ typedef int (*section_fn) (struct format_instance * fid, struct dm_pool * mem,
 			   struct volume_group * vg, struct config_node * pvn,
 			   struct config_node * vgn,
 			   struct dm_hash_table * pv_hash,
+			   struct dm_hash_table * lv_hash,
 			   unsigned *scan_done_once,
 			   unsigned report_missing_devices);
 
@@ -155,7 +156,9 @@ static int _read_flag_config(struct config_node *n, uint64_t *status, int type)
 static int _read_pv(struct format_instance *fid, struct dm_pool *mem,
 		    struct volume_group *vg, struct config_node *pvn,
 		    struct config_node *vgn __attribute((unused)),
-		    struct dm_hash_table *pv_hash, unsigned *scan_done_once,
+		    struct dm_hash_table *pv_hash,
+		    struct dm_hash_table *lv_hash __attribute((unused)),
+		    unsigned *scan_done_once,
 		    unsigned report_missing_devices)
 {
 	struct physical_volume *pv;
@@ -495,6 +498,7 @@ static int _read_lvnames(struct format_instance *fid __attribute((unused)),
 			 struct volume_group *vg, struct config_node *lvn,
 			 struct config_node *vgn __attribute((unused)),
 			 struct dm_hash_table *pv_hash __attribute((unused)),
+			 struct dm_hash_table *lv_hash,
 			 unsigned *scan_done_once __attribute((unused)),
 			 unsigned report_missing_devices __attribute((unused)))
 {
@@ -555,6 +559,9 @@ static int _read_lvnames(struct format_instance *fid __attribute((unused)),
 		return 0;
 	}
 
+	if (!dm_hash_insert(lv_hash, lv->name, lv))
+		return_0;
+
 	return link_lv_to_vg(vg, lv);
 }
 
@@ -563,19 +570,17 @@ static int _read_lvsegs(struct format_instance *fid __attribute((unused)),
 			struct volume_group *vg, struct config_node *lvn,
 			struct config_node *vgn __attribute((unused)),
 			struct dm_hash_table *pv_hash,
+			struct dm_hash_table *lv_hash,
 			unsigned *scan_done_once __attribute((unused)),
 			unsigned report_missing_devices __attribute((unused)))
 {
 	struct logical_volume *lv;
-	struct lv_list *lvl;
 
-	if (!(lvl = find_lv_in_vg(vg, lvn->key))) {
+	if (!(lv = dm_hash_lookup(lv_hash, lvn->key))) {
 		log_error("Lost logical volume reference %s", lvn->key);
 		return 0;
 	}
 
-	lv = lvl->lv;
-
 	if (!(lvn = lvn->child)) {
 		log_error("Empty logical volume section.");
 		return 0;
@@ -617,7 +622,9 @@ static int _read_sections(struct format_instance *fid,
 			  const char *section, section_fn fn,
 			  struct dm_pool *mem,
 			  struct volume_group *vg, struct config_node *vgn,
-			  struct dm_hash_table *pv_hash, int optional,
+			  struct dm_hash_table *pv_hash,
+			  struct dm_hash_table *lv_hash,
+			  int optional,
 			  unsigned *scan_done_once)
 {
 	struct config_node *n;
@@ -634,7 +641,7 @@ static int _read_sections(struct format_instance *fid,
 	}
 
 	for (n = n->child; n; n = n->sib) {
-		if (!fn(fid, mem, vg, n, vgn, pv_hash, scan_done_once, report_missing_devices))
+		if (!fn(fid, mem, vg, n, vgn, pv_hash, lv_hash, scan_done_once, report_missing_devices))
 			return_0;
 	}
 
@@ -647,7 +654,7 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 {
 	struct config_node *vgn, *cn;
 	struct volume_group *vg;
-	struct dm_hash_table *pv_hash = NULL;
+	struct dm_hash_table *pv_hash = NULL, *lv_hash = NULL;
 	struct dm_pool *mem = dm_pool_create("lvm2 vg_read", VG_MEMPOOL_CHUNK);
 	unsigned scan_done_once = use_cached_pvs;
 
@@ -752,7 +759,7 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 
 	dm_list_init(&vg->pvs);
 	if (!_read_sections(fid, "physical_volumes", _read_pv, mem, vg,
-			    vgn, pv_hash, 0, &scan_done_once)) {
+			    vgn, pv_hash, lv_hash, 0, &scan_done_once)) {
 		log_error("Couldn't find all physical volumes for volume "
 			  "group %s.", vg->name);
 		goto bad;
@@ -769,15 +776,24 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 		goto bad;
 	}
 
+	/*
+	 * The lv hash memoises the lv section names -> lv
+	 * structures.
+	 */
+	if (!(lv_hash = dm_hash_create(32))) {
+		log_error("Couldn't create hash table.");
+		goto bad;
+	}
+
 	if (!_read_sections(fid, "logical_volumes", _read_lvnames, mem, vg,
-			    vgn, pv_hash, 1, NULL)) {
+			    vgn, pv_hash, lv_hash, 1, NULL)) {
 		log_error("Couldn't read all logical volume names for volume "
 			  "group %s.", vg->name);
 		goto bad;
 	}
 
 	if (!_read_sections(fid, "logical_volumes", _read_lvsegs, mem, vg,
-			    vgn, pv_hash, 1, NULL)) {
+			    vgn, pv_hash, lv_hash, 1, NULL)) {
 		log_error("Couldn't read all logical volumes for "
 			  "volume group %s.", vg->name);
 		goto bad;
@@ -790,6 +806,7 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 	}
 
 	dm_hash_destroy(pv_hash);
+	dm_hash_destroy(lv_hash);
 
 	/*
 	 * Finished.
@@ -800,6 +817,9 @@ static struct volume_group *_read_vg(struct format_instance *fid,
 	if (pv_hash)
 		dm_hash_destroy(pv_hash);
 
+	if (lv_hash)
+		dm_hash_destroy(lv_hash);
+
 	dm_pool_destroy(mem);
 	return NULL;
 }
-- 
1.7.0.3

[PATCH 2/5] Remove vg_validate call when parsing cached metadata.

[Milan Broz]

vg_validate call is an adept to optimisation, it is very
ineeficient and slow.

Anyway, we should call it only before writing data to disk.

The call in lvmcache was just temporary validation,
we realy do not need to revalidate cached metadata
every time.
(Actually, I added that there just to prove that cache works
properly and forgot to remove it.)

Patch removes it from lvmcache completely, this can hit only
internal bug in export function (and this bug must
be detected in any vg_write call anyway before).

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 lib/cache/lvmcache.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index 0ce034a..9caa216 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -610,8 +610,7 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
 						      vgid, NULL)))
 		return_NULL;
 
-	if (!(vg = import_vg_from_buffer(vginfo->vgmetadata, fid)) ||
-	    !vg_validate(vg)) {
+	if (!(vg = import_vg_from_buffer(vginfo->vgmetadata, fid))) {
 		_free_cached_vgmetadata(vginfo);
 		vg_release(vg);
 		return_NULL;
-- 
1.7.0.3

[PATCH 3/5] Optimise PV segments search.

[Milan Broz]

The function find_peg_by_pe is incredibly inefficient
for Pvs with many segments.

In shiny future there should be binary (or interval) tree
instead of sorted linked list (volunteers?).

Anyway, for now, we can use dirty trick here to optimise this case:

 - Allocations are usually applied from the beginning
 of PV (we have no alloocation policy which allocates areas
 "backwards")

 - The only user of find_peg_by_pe is pv_split_segment()
 call. In *most* cases it need to split *last* PV segment.

So if we search sorted pv segment list backwards, we
hit the requested segment immediatelly.

This patch applies this tiny change.
(and saves >30% of processing time when >3000LVs segments are on one PV!)

To discourage using this inefficient function from other code,
it is moved to pv_manip.c and used static for now:-)

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 lib/metadata/metadata.c |   12 ------------
 lib/metadata/metadata.h |    3 ---
 lib/metadata/pv_manip.c |   14 ++++++++++++++
 3 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c
index 020e897..c52acb8 100644
--- a/lib/metadata/metadata.c
+++ b/lib/metadata/metadata.c
@@ -1799,18 +1799,6 @@ struct lv_segment *first_seg(const struct logical_volume *lv)
 	return NULL;
 }
 
-/* Find segment at a given physical extent in a PV */
-struct pv_segment *find_peg_by_pe(const struct physical_volume *pv, uint32_t pe)
-{
-	struct pv_segment *peg;
-
-	dm_list_iterate_items(peg, &pv->segments)
-		if (pe >= peg->pe && pe < peg->pe + peg->len)
-			return peg;
-
-	return NULL;
-}
-
 int vg_remove_mdas(struct volume_group *vg)
 {
 	struct metadata_area *mda;
diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h
index 542be6e..782d300 100644
--- a/lib/metadata/metadata.h
+++ b/lib/metadata/metadata.h
@@ -308,9 +308,6 @@ struct pv_list *find_pv_in_pv_list(const struct dm_list *pl,
 /* Find LV segment containing given LE */
 struct lv_segment *find_seg_by_le(const struct logical_volume *lv, uint32_t le);
 
-/* Find PV segment containing given LE */
-struct pv_segment *find_peg_by_pe(const struct physical_volume *pv, uint32_t pe);
-
 /*
  * Remove a dev_dir if present.
  */
diff --git a/lib/metadata/pv_manip.c b/lib/metadata/pv_manip.c
index 1b0a457..b80c3f1 100644
--- a/lib/metadata/pv_manip.c
+++ b/lib/metadata/pv_manip.c
@@ -78,6 +78,20 @@ int peg_dup(struct dm_pool *mem, struct dm_list *peg_new, struct dm_list *peg_ol
 	return 1;
 }
 
+/* Find segment at a given physical extent in a PV */
+static struct pv_segment *find_peg_by_pe(const struct physical_volume *pv,
+					 uint32_t pe)
+{
+	struct pv_segment *peg;
+
+	/* search backwards to optimise mostly used last segment split */
+	dm_list_iterate_back_items(peg, &pv->segments)
+		if (pe >= peg->pe && pe < peg->pe + peg->len)
+			return peg;
+
+	return NULL;
+}
+
 /*
  * Split peg at given extent.
  * Second part is always deallocated.
-- 
1.7.0.3

[PATCH 4/5] Do not traverse PV segment list twice.

[Milan Broz]

In addition to previous patch, we really do not need
to search for segment which was just allocated in
split request.

Make pv_split_segment function return newly allocated
(split) segment also.

(So after this patch, there is only one user
of slow find_peg_by_pe).

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 lib/metadata/pv_alloc.h |    3 ++-
 lib/metadata/pv_manip.c |   39 +++++++++++++++++++++++----------------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/lib/metadata/pv_alloc.h b/lib/metadata/pv_alloc.h
index 7c67b2c..601bbf1 100644
--- a/lib/metadata/pv_alloc.h
+++ b/lib/metadata/pv_alloc.h
@@ -20,7 +20,8 @@ struct pv_segment *assign_peg_to_lvseg(struct physical_volume *pv, uint32_t pe,
 				       uint32_t area_len,
 				       struct lv_segment *seg,
 				       uint32_t area_num);
-int pv_split_segment(struct physical_volume *pv, uint32_t pe);
+int pv_split_segment(struct physical_volume *pv, uint32_t pe,
+		     struct pv_segment **peg_allocated);
 int release_pv_segment(struct pv_segment *peg, uint32_t area_reduction);
 int check_pv_segments(struct volume_group *vg);
 void merge_pv_segments(struct pv_segment *peg1, struct pv_segment *peg2);
diff --git a/lib/metadata/pv_manip.c b/lib/metadata/pv_manip.c
index b80c3f1..bd3f488 100644
--- a/lib/metadata/pv_manip.c
+++ b/lib/metadata/pv_manip.c
@@ -96,15 +96,16 @@ static struct pv_segment *find_peg_by_pe(const struct physical_volume *pv,
  * Split peg at given extent.
  * Second part is always deallocated.
  */
-static int _pv_split_segment(struct physical_volume *pv, struct pv_segment *peg,
-			     uint32_t pe)
+static struct pv_segment *_pv_split_segment(struct physical_volume *pv,
+					    struct pv_segment *peg,
+					    uint32_t pe)
 {
 	struct pv_segment *peg_new;
 
 	if (!(peg_new = _alloc_pv_segment(pv->fmt->cmd->mem, peg->pv, pe,
 					  peg->len + peg->pe - pe,
 					  NULL, 0)))
-		return_0;
+		return_NULL;
 
 	peg->len = peg->len - peg_new->len;
 
@@ -115,18 +116,19 @@ static int _pv_split_segment(struct physical_volume *pv, struct pv_segment *peg,
 		peg->lvseg->lv->vg->free_count += peg_new->len;
 	}
 
-	return 1;
+	return peg_new;
 }
 
 /*
  * Ensure there is a PV segment boundary at the given extent.
  */
-int pv_split_segment(struct physical_volume *pv, uint32_t pe)
+int pv_split_segment(struct physical_volume *pv, uint32_t pe,
+		     struct pv_segment **peg_allocated)
 {
-	struct pv_segment *peg;
+	struct pv_segment *peg, *peg_new = NULL;
 
 	if (pe == pv->pe_count)
-		return 1;
+		goto out;
 
 	if (!(peg = find_peg_by_pe(pv, pe))) {
 		log_error("Segment with extent %" PRIu32 " in PV %s not found",
@@ -135,11 +137,16 @@ int pv_split_segment(struct physical_volume *pv, uint32_t pe)
 	}
 
 	/* This is a peg start already */
-	if (pe == peg->pe)
-		return 1;
+	if (pe == peg->pe) {
+		peg_new = peg;
+		goto out;
+	}
 
-	if (!_pv_split_segment(pv, peg, pe))
+	if (!(peg_new = _pv_split_segment(pv, peg, pe)))
 		return_0;
+out:
+	if (peg_allocated)
+		*peg_allocated = peg_new;
 
 	return 1;
 }
@@ -154,17 +161,17 @@ struct pv_segment *assign_peg_to_lvseg(struct physical_volume *pv,
 				       struct lv_segment *seg,
 				       uint32_t area_num)
 {
-	struct pv_segment *peg;
+	struct pv_segment *peg = NULL;
 
 	/* Missing format1 PV */
 	if (!pv)
 		return &null_pv_segment;
 
-	if (!pv_split_segment(pv, pe) ||
-	    !pv_split_segment(pv, pe + area_len))
+	if (!pv_split_segment(pv, pe, &peg) ||
+	    !pv_split_segment(pv, pe + area_len, NULL))
 		return_NULL;
 
-	if (!(peg = find_peg_by_pe(pv, pe))) {
+	if (!peg) {
 		log_error("Missing PV segment on %s at %u.",
 			  pv_dev_name(pv), pe);
 		return NULL;
@@ -200,7 +207,7 @@ int release_pv_segment(struct pv_segment *peg, uint32_t area_reduction)
 	}
 
 	if (!pv_split_segment(peg->pv, peg->pe + peg->lvseg->area_len -
-				       area_reduction))
+				       area_reduction, NULL))
 		return_0;
 
 	return 1;
@@ -373,7 +380,7 @@ static int _reduce_pv(struct physical_volume *pv, struct volume_group *vg, uint3
 		}
 	}
 
-	if (!pv_split_segment(pv, new_pe_count))
+	if (!pv_split_segment(pv, new_pe_count, NULL))
 		return_0;
 
 	dm_list_iterate_items_safe(peg, pegt, &pv->segments) {
-- 
1.7.0.3

[PATCH 5/5] Fix all segments memory is allocated from vg private mempool.

[Milan Broz]

Physical segments were still allocated from global
command context mempool.

This leads to very high memory usage when
activating large VG (vgchange).
(Memory usage was about 2G when >3000LVs).

Fix it by properly using vg->vgmem private pool,
so all the memory is released early.

New memory pool parameter is needed here for pv_split_segment
function.

Also fix the same problem in some minor allocations
(vg description, lv segment split).

Signed-off-by: Milan Broz <mbroz@redhat.com>
---
 lib/format_text/import.c |    2 +-
 lib/metadata/lv_manip.c  |    2 +-
 lib/metadata/merge.c     |    4 ++--
 lib/metadata/pv_alloc.h  |    3 ++-
 lib/metadata/pv_manip.c  |   21 ++++++++++++---------
 5 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/lib/format_text/import.c b/lib/format_text/import.c
index 03ff990..f686418 100644
--- a/lib/format_text/import.c
+++ b/lib/format_text/import.c
@@ -114,7 +114,7 @@ struct volume_group *text_vg_import_fd(struct format_instance *fid,
 		if (!(vg = (*vsn)->read_vg(fid, cft, 0)))
 			goto_out;
 
-		(*vsn)->read_desc(fid->fmt->cmd->mem, cft, when, desc);
+		(*vsn)->read_desc(vg->vgmem, cft, when, desc);
 		break;
 	}
 
diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c
index c9d1759..650bc83 100644
--- a/lib/metadata/lv_manip.c
+++ b/lib/metadata/lv_manip.c
@@ -47,7 +47,7 @@ int add_seg_to_segs_using_this_lv(struct logical_volume *lv,
 	log_very_verbose("Adding %s:%" PRIu32 " as an user of %s",
 			 seg->lv->name, seg->le, lv->name);
 
-	if (!(sl = dm_pool_zalloc(lv->vg->cmd->mem, sizeof(*sl)))) {
+	if (!(sl = dm_pool_zalloc(lv->vg->vgmem, sizeof(*sl)))) {
 		log_error("Failed to allocate segment list");
 		return 0;
 	}
diff --git a/lib/metadata/merge.c b/lib/metadata/merge.c
index 66e9ce0..bd65c69 100644
--- a/lib/metadata/merge.c
+++ b/lib/metadata/merge.c
@@ -268,7 +268,7 @@ static int _lv_split_segment(struct logical_volume *lv, struct lv_segment *seg,
 	}
 
 	/* Clone the existing segment */
-	if (!(split_seg = alloc_lv_segment(lv->vg->cmd->mem, seg->segtype,
+	if (!(split_seg = alloc_lv_segment(lv->vg->vgmem, seg->segtype,
 					   seg->lv, seg->le, seg->len,
 					   seg->status, seg->stripe_size,
 					   seg->log_lv,
@@ -279,7 +279,7 @@ static int _lv_split_segment(struct logical_volume *lv, struct lv_segment *seg,
 		return 0;
 	}
 
-	if (!str_list_dup(lv->vg->cmd->mem, &split_seg->tags, &seg->tags)) {
+	if (!str_list_dup(lv->vg->vgmem, &split_seg->tags, &seg->tags)) {
 		log_error("LV segment tags duplication failed");
 		return 0;
 	}
diff --git a/lib/metadata/pv_alloc.h b/lib/metadata/pv_alloc.h
index 601bbf1..ebf6561 100644
--- a/lib/metadata/pv_alloc.h
+++ b/lib/metadata/pv_alloc.h
@@ -20,7 +20,8 @@ struct pv_segment *assign_peg_to_lvseg(struct physical_volume *pv, uint32_t pe,
 				       uint32_t area_len,
 				       struct lv_segment *seg,
 				       uint32_t area_num);
-int pv_split_segment(struct physical_volume *pv, uint32_t pe,
+int pv_split_segment(struct dm_pool *mem,
+		     struct physical_volume *pv, uint32_t pe,
 		     struct pv_segment **peg_allocated);
 int release_pv_segment(struct pv_segment *peg, uint32_t area_reduction);
 int check_pv_segments(struct volume_group *vg);
diff --git a/lib/metadata/pv_manip.c b/lib/metadata/pv_manip.c
index bd3f488..a4ca7a1 100644
--- a/lib/metadata/pv_manip.c
+++ b/lib/metadata/pv_manip.c
@@ -96,13 +96,14 @@ static struct pv_segment *find_peg_by_pe(const struct physical_volume *pv,
  * Split peg at given extent.
  * Second part is always deallocated.
  */
-static struct pv_segment *_pv_split_segment(struct physical_volume *pv,
+static struct pv_segment *_pv_split_segment(struct dm_pool *mem,
+					    struct physical_volume *pv,
 					    struct pv_segment *peg,
 					    uint32_t pe)
 {
 	struct pv_segment *peg_new;
 
-	if (!(peg_new = _alloc_pv_segment(pv->fmt->cmd->mem, peg->pv, pe,
+	if (!(peg_new = _alloc_pv_segment(mem, peg->pv, pe,
 					  peg->len + peg->pe - pe,
 					  NULL, 0)))
 		return_NULL;
@@ -122,7 +123,8 @@ static struct pv_segment *_pv_split_segment(struct physical_volume *pv,
 /*
  * Ensure there is a PV segment boundary at the given extent.
  */
-int pv_split_segment(struct physical_volume *pv, uint32_t pe,
+int pv_split_segment(struct dm_pool *mem,
+		     struct physical_volume *pv, uint32_t pe,
 		     struct pv_segment **peg_allocated)
 {
 	struct pv_segment *peg, *peg_new = NULL;
@@ -142,7 +144,7 @@ int pv_split_segment(struct physical_volume *pv, uint32_t pe,
 		goto out;
 	}
 
-	if (!(peg_new = _pv_split_segment(pv, peg, pe)))
+	if (!(peg_new = _pv_split_segment(mem, pv, peg, pe)))
 		return_0;
 out:
 	if (peg_allocated)
@@ -167,8 +169,8 @@ struct pv_segment *assign_peg_to_lvseg(struct physical_volume *pv,
 	if (!pv)
 		return &null_pv_segment;
 
-	if (!pv_split_segment(pv, pe, &peg) ||
-	    !pv_split_segment(pv, pe + area_len, NULL))
+	if (!pv_split_segment(seg->lv->vg->vgmem, pv, pe, &peg) ||
+	    !pv_split_segment(seg->lv->vg->vgmem, pv, pe + area_len, NULL))
 		return_NULL;
 
 	if (!peg) {
@@ -206,8 +208,9 @@ int release_pv_segment(struct pv_segment *peg, uint32_t area_reduction)
 		return 1;
 	}
 
-	if (!pv_split_segment(peg->pv, peg->pe + peg->lvseg->area_len -
-				       area_reduction, NULL))
+	if (!pv_split_segment(peg->lvseg->lv->vg->vgmem,
+			      peg->pv, peg->pe + peg->lvseg->area_len -
+			      area_reduction, NULL))
 		return_0;
 
 	return 1;
@@ -380,7 +383,7 @@ static int _reduce_pv(struct physical_volume *pv, struct volume_group *vg, uint3
 		}
 	}
 
-	if (!pv_split_segment(pv, new_pe_count, NULL))
+	if (!pv_split_segment(vg->vgmem, pv, new_pe_count, NULL))
 		return_0;
 
 	dm_list_iterate_items_safe(peg, pegt, &pv->segments) {
-- 
1.7.0.3

[PATCH 2/5] Remove vg_validate call when parsing cached metadata.

[Zdenek Kabelac]

On 29.3.2010 15:12, Milan Broz wrote:
> vg_validate call is an adept to optimisation, it is very
> ineeficient and slow.
> 
> Anyway, we should call it only before writing data to disk.
> 
> The call in lvmcache was just temporary validation,
> we realy do not need to revalidate cached metadata
> every time.
> (Actually, I added that there just to prove that cache works
> properly and forgot to remove it.)
> 
> Patch removes it from lvmcache completely, this can hit only
> internal bug in export function (and this bug must
> be detected in any vg_write call anyway before).
> 
> Signed-off-by: Milan Broz <mbroz@redhat.com>
> ---
>  lib/cache/lvmcache.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
> index 0ce034a..9caa216 100644
> --- a/lib/cache/lvmcache.c
> +++ b/lib/cache/lvmcache.c
> @@ -610,8 +610,7 @@ struct volume_group *lvmcache_get_vg(const char *vgid, unsigned precommitted)
>  						      vgid, NULL)))
>  		return_NULL;
>  
> -	if (!(vg = import_vg_from_buffer(vginfo->vgmetadata, fid)) ||
> -	    !vg_validate(vg)) {
> +	if (!(vg = import_vg_from_buffer(vginfo->vgmetadata, fid))) {
>  		_free_cached_vgmetadata(vginfo);
>  		vg_release(vg);
>  		return_NULL;


Ack


We control metadata before write.

Zdenek

[PATCH 0/5] Fix some scaling problems for large VGs (many LVs)

[Zdenek Kabelac]

..
> 
> Milan Broz (5):
>   Use hash table for quick lv reference when reading metadata.
>   Remove vg_validate call when parsing cached metadata.
>   Optimise PV segments search.
>   Do not traverse PV segment list twice.
>   Fix all segments memory is allocated from vg private mempool.
> 
>  lib/cache/lvmcache.c          |    3 +-
>  lib/format_text/import.c      |    2 +-
>  lib/format_text/import_vsn1.c |   42 +++++++++++++++++++++-------
>  lib/metadata/lv_manip.c       |    2 +-
>  lib/metadata/merge.c          |    4 +-
>  lib/metadata/metadata.c       |   12 --------
>  lib/metadata/metadata.h       |    3 --
>  lib/metadata/pv_alloc.h       |    4 ++-
>  lib/metadata/pv_manip.c       |   60 ++++++++++++++++++++++++++++------------
>  9 files changed, 81 insertions(+), 51 deletions(-)
> 
> 

Patch set goes to right direction - but still there is probably more complex
solution - In activation we could keep locked & parsed VG data in volume_group
structure in memory - instead of parsing them for each LV again.
(for the case of having like 3000LV in one VG)

Zdenek

[PATCH 0/5] Fix some scaling problems for large VGs (many LVs)

[Alasdair G Kergon]

On Mon, Mar 29, 2010 at 03:12:53PM +0200, Milan Broz wrote:
> This patches fixes some problems in code when processing large VGs.
 
Ack.

Alasdair