Fix max_parallel calculation

PATCH New Jun'ichi Nomura 1 participant 1 message Aug 29, 2006

lore

Share & tools

lore

Jun'ichi Nomura

author

2006-08-29 17:12 UTC #0 lore raw

PATCH 1/1 Fix max_parallel calculation

Hi,

In _find_parallel_space(), there is 2 mistakes in calculation of max_parallel (and *allocated).

1. max_parallel and *allocated are number of extents multiplied by

     ah->area_multiple.
     So it needs to be divided by ah->area_multiple when it's used for
     stripe length calculation

2. max_parallel specifies the total number of extents, not the newly allocated one.
So it doesn't need to be offset by starting le.

As a result, allocation fails unexpectedly.

Typical failure cases:
1) lvcreate --alloc contiguous fails for striped LV if requested LV size is larger than a PV size
2) lvconvert -m to add mirror to existing LV fails if the existing LV has more than one segments

Attached patch fixes them.
Applicable to 2.02.09.

Thanks,

Signature · 1 line

Jun'ichi Nomura, NEC Corporation of America

In _find_parallel_space(), there is 2 mistakes in calculation of max_parallel (and *allocated).

1. max_parallel and *allocated are number of extents multiplied by

     ah->area_multiple.
     So it needs to be divided by ah->area_multiple when it's used for
     stripe length calculation

2. max_parallel specifies the total number of extents, not the newly allocated one.
So it doesn't need to be offset by starting le.

Attached patch fixes them.
Otherwise, allocation fails unexpectedly.

lib/metadata/lv_manip.c +12 -5

diff -urp LVM2.01/lib/metadata/lv_manip.c LVM2.02/lib/metadata/lv_manip.c
@@ -715,6 +715,12 @@ static int _find_parallel_space(struct a
 	do {
 		ix = 0;
 
+		/*
+		 * max_parallel and *allocated are number of extents
+		 * multiplied by ah->area_multiple.
+		 * So they should be divided by ah->area_multiple before
+		 * using for calcualtion of a stripe length.
+		 */
 		parallel_pvs = NULL;
 		max_parallel = needed;

@@ -723,11 +729,12 @@ static int _find_parallel_space(struct a
 		 * the maximum we can allocate in one go accordingly.
 		 */
 		if (ah->parallel_areas) {
+			next_le = (prev_lvseg ? prev_lvseg->le + prev_lvseg->len : 0) + (*allocated / ah->area_multiple);
 			list_iterate_items(spvs, ah->parallel_areas) {
-				next_le = (prev_lvseg ? prev_lvseg->le + prev_lvseg->len : 0) + *allocated;
-				if (next_le >= spvs->le) {
-					if (next_le + max_parallel > spvs->le + spvs->len)
-						max_parallel = (spvs->le + spvs->len - next_le) * ah->area_multiple;
+				if (next_le >= spvs->le &&
+				    next_le < spvs->le + spvs->len) {
+					if (max_parallel / ah->area_multiple > spvs->le + spvs->len)
+						max_parallel = (spvs->le + spvs->len) * ah->area_multiple;
 					parallel_pvs = &spvs->pvs;
 					break;
 				}

@@ -773,7 +780,7 @@ static int _find_parallel_space(struct a
 				}
 
 				/* Is it big enough on its own? */
-				if ((pva->count < max_parallel - *allocated) &&
+				if ((pva->count < (max_parallel - *allocated) / ah->area_multiple) &&
 				    ((!can_split && !ah->log_count) ||
 				     (already_found_one &&
 				      !(alloc == ALLOC_ANYWHERE))))