[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[jan@gondor.com: Re: [linux-lvm] pvmove hangs]



...forwarding this to the list, to avoid duplication of work in case
somebody else but Alasdair working on that matter.

The warnings still apply, it's only marginally tested (I actually loaded
the modules and did a pvmove, but did nothing else with them)

This patch would not have been possible without Alasdair finding the
cause of the problem.

Jan

--- Begin Message ---
On Thu, Aug 21, 2003 at 11:01:47PM +0100, Alasdair G Kergon wrote:
> [off list]
ok

I thought about the realloc problem. It's basically only a problem
because 
a) dm_table is a table and not a linked list (so it needs to move when
it grows)

and

b) the elements contained in dm_table, namely dm_target object, are
directly referenced by the mirror_set structures, which do not get
updated on dm_table reallocs.


So, to solve the problem, one could change a) or b), or use the standard
solution in computer science: add another layer of indirection.

I think the last option would be quite easy to implement: 
In struct dm_table, change dm_target *targets to dm_target **targets,
and make it an array (table) of pointers instead of a table of
dm_targets.

So if you resize the table, the actual dm_target structs don't move in
memory. So b) is not a problem any more. And the array of pointers can
be moved without problems, because it's only accessed from the dm_table
structure, which gets updated properly.

As the dm code is well structured, that change would be localised to
dm-table.c. See the attached patch for an example. But be warned. I just
wrote it and checked that it compiles, it may not even boot, or may
cause severe corruption...

Jan

--- linux-2.4.21/drivers/md/dm-table.c.orig	2003-08-22 01:42:35.000000000 +0200
+++ linux-2.4.21/drivers/md/dm-table.c	2003-08-22 02:10:29.000000000 +0200
@@ -30,7 +30,7 @@
 	unsigned int num_targets;
 	unsigned int num_allocated;
 	sector_t *highs;
-	struct dm_target *targets;
+	struct dm_target **targets;
 
 	/*
 	 * Indicates the rw permissions for the new logical
@@ -120,18 +120,19 @@
 static int alloc_targets(struct dm_table *t, unsigned int num)
 {
 	sector_t *n_highs;
-	struct dm_target *n_targets;
+	struct dm_target **n_targets;
 	int n = t->num_targets;
+	unsigned int i;
 
 	/*
 	 * Allocate both the target array and offset array at once.
 	 */
-	n_highs = (sector_t *) vcalloc(sizeof(struct dm_target) +
+	n_highs = (sector_t *) vcalloc(sizeof(struct dm_target *) +
 				       sizeof(sector_t), num);
 	if (!n_highs)
 		return -ENOMEM;
 
-	n_targets = (struct dm_target *) (n_highs + num);
+	n_targets = (struct dm_target **) (n_highs + num);
 
 	if (n) {
 		memcpy(n_highs, t->highs, sizeof(*n_highs) * n);
@@ -141,6 +142,18 @@
 	memset(n_highs + n, -1, sizeof(*n_highs) * (num - n));
 	vfree(t->highs);
 
+	for(i=n; i<num; i++) {
+		*(n_targets+i)=vcalloc(sizeof(struct dm_target),1);
+		if(!n_highs) break;
+	}
+	if(i!=num) { // Could not alloc all dm_targets
+		for(i--;i>n; i--) {
+			vfree(*(n_targets+i));
+		}
+		vfree(n_highs);
+		return -ENOMEM;
+	}
+
 	t->num_allocated = num;
 	t->highs = n_highs;
 	t->targets = n_targets;
@@ -192,12 +205,13 @@
 
 	/* free the targets */
 	for (i = 0; i < t->num_targets; i++) {
-		struct dm_target *tgt = t->targets + i;
+		struct dm_target *tgt = *(t->targets + i);
 
 		if (tgt->type->dtr)
 			tgt->type->dtr(tgt);
 
 		dm_put_target_type(tgt->type);
+		vfree(tgt);
 	}
 
 	vfree(t->highs);
@@ -437,7 +451,7 @@
 	if (!table->num_targets)
 		return !ti->begin;
 
-	prev = &table->targets[table->num_targets - 1];
+	prev = table->targets[table->num_targets - 1];
 	return (ti->begin == (prev->begin + prev->len));
 }
 
@@ -521,7 +535,7 @@
 	if ((r = check_space(t)))
 		return r;
 
-	tgt = t->targets + t->num_targets;
+	tgt = *(t->targets + t->num_targets);
 	memset(tgt, 0, sizeof(*tgt));
 
 	tgt->type = dm_get_target_type(type);
@@ -640,7 +654,7 @@
 	if (index > t->num_targets)
 		return NULL;
 
-	return t->targets + index;
+	return *(t->targets + index);
 }
 
 /*
@@ -660,7 +674,7 @@
 				break;
 	}
 
-	return &t->targets[(KEYS_PER_NODE * n) + k];
+	return t->targets[(KEYS_PER_NODE * n) + k];
 }
 
 unsigned int dm_table_get_num_targets(struct dm_table *t)
@@ -683,7 +697,7 @@
 	int i;
 
 	for (i = 0; i < t->num_targets; i++) {
-		struct dm_target *ti = t->targets + i;
+		struct dm_target *ti = *(t->targets + i);
 
 		if (ti->type->suspend)
 			ti->type->suspend(ti);
@@ -695,7 +709,7 @@
 	int i;
 
 	for (i = 0; i < t->num_targets; i++) {
-		struct dm_target *ti = t->targets + i;
+		struct dm_target *ti = *(t->targets + i);
 
 		if (ti->type->resume)
 			ti->type->resume(ti);

Attachment: pgp00003.pgp
Description: PGP signature


--- End Message ---

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]