[PATCH 0/8] OLCE Raid5/0 single array for external meta

[PATCH 0/8] OLCE Raid5/0 single array for external meta

[Adam Kwolek]

Thank you for review previous OLCE series. 

I've reviewed metadata update method to avoid as you pointed layering violation.

The previous approach was prepared to minimize places to do the same things.
1. one for metadata update via mdmon
2. second for direct metadata update by mdata (no mdmon case)

Current approach, is a try for making one updates processing patch for mdmon and "no mdmon" case.
1. mdmon case:
   a) reshape_super()
           - prepare update
           - append_metadata_update()
           - apply last update in queue (top of '->updates')  using prepare/process update functions
                update is not removed from queue
   b) sync_metadata() to flush update to mdmon

2. no mdmon case
  a) reshape_super()
     Works in the same way as in "mdmon case"
   b) sync_metadata() to flush metadata (already updated) to array
     if there is any update in update queue it is released, it is assumed that only one direct update
     is curently supported.


This mechanism is implemented in patches:
      imsm: FIX: old devices memory has to be released
      imsm: FIX: local mdadm update shouldn't be done in update creation function.
      imsm: FIX: mdadm should process local data
      FIX: put update in to queue for local meta update

The rest are mainly bug fixes.

The one patch "FIX: reload metadata for container operation" needs more comments.
So far I've to reload metadata in reshape_container(), I've learned that you prefer reload metadat on the end of reshape_array().
I think that we should proceed in the same way when reshape_array() is forked externally and code returns to caller.

I'm waiting for your review, meanwhile I'll make some work for next arrays in container reshape

BR
Adam




---

Adam Kwolek (8):
      FIX: reload metadata for container operation
      Finalize reshape after adding disks to array
      imsm: FIX: spares are not counted
      FIX: Cannot load container information
      imsm: FIX: old devices memory has to be released
      imsm: FIX: local mdadm update shouldn't be done in update creation function.
      imsm: FIX: mdadm should process local data
      FIX: put update in to queue for local meta update


 Grow.c        |   28 ++++++++++++++--
 monitor.c     |   11 ++++++
 super-intel.c |  101 ++++++++++++++++++---------------------------------------
 util.c        |   10 +++++-
 4 files changed, 76 insertions(+), 74 deletions(-)

-- 
Signature

[PATCH 1/8] FIX: put update in to queue for local meta update

[Adam Kwolek]

When mdmon is not active (raid0 case) update has to be applied locally by mdadm.
In such case during append_metadata_update() update_tail doesn't exist
and update can be directly put in to update for local processing.

After this reshape_super() can directly call prepare_update() and process_update()
to update anchor in the same way as mdmon does it. space list cleanup is required only.
sync_metadata() writes metadata to array (by mdadm)

When mdmon exists, local update is applied but not flushed to array, it is sent to mdmon
and from mdmon update is flushed to disk. This allows us to avoid anchor reloading.

It is recommended to use local updates in mdadm (without anchor reload) with mdmon blocked/array frozen/.
This makes us sure that there is no other changes that should be taken under consideration
during processing in mdadm.

For local updates single update queue is supported.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 Grow.c |    7 ++++++-
 util.c |   10 ++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/Grow.c b/Grow.c
index 898eb3e..c2e84f3 100644
--- a/Grow.c
+++ b/Grow.c
@@ -591,8 +591,13 @@ static void sync_metadata(struct supertype *st)
 		if (st->update_tail) {
 			flush_metadata_updates(st);
 			st->update_tail = &st->updates;
-		} else
+		} else {
 			st->ss->sync_metadata(st);
+			if (st->updates) {
+				free(st->updates);
+				st->updates = NULL;
+			}
+		}
 	}
 }
 
diff --git a/util.c b/util.c
index 10d2140..9e3f13f 100644
--- a/util.c
+++ b/util.c
@@ -1889,8 +1889,14 @@ void append_metadata_update(struct supertype *st, void *buf, int len)
 	mu->space = NULL;
 	mu->space_list = NULL;
 	mu->next = NULL;
-	*st->update_tail = mu;
-	st->update_tail = &mu->next;
+	if (st->update_tail) {
+		*st->update_tail = mu;
+		st->update_tail = &mu->next;
+	} else {
+		if (st->updates)
+			free(st->updates);
+		st->updates = mu;
+	}
 }
 #endif /* MDASSEMBLE */
 

[PATCH 2/8] imsm: FIX: mdadm should process local data

[Adam Kwolek]

When update is created by mdadm, local information should be updated also.
This makes us to prepare one update for mdmon and second "update" to maintain local changes.
we can use prepared update for "local/mdadm" metadata update purposes.
We have 2 cases:
1. when metadata is updated by mdmon, we avoid metadata reloading in mdadm.
   we proceed the same updtate 2 times:
	- one time in mdadm for "local update"
        - second time in mdmon for real metadat update
2. when metadata is updated by mdadm (no mdmon running) updates are processed in the same
   way.
	- one time in mdadm for "local update"
	- there is no "second time" update but mdadm just flushes
          metadata to array
   This let us to avoid code duplication by using prepare and process update functions
   as for update via mdmon. This makes update preparing mdmon independent
   and there is no need to maintain the same thing in 2 places in code.


Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 super-intel.c |   82 ++++++++++++++++++---------------------------------------
 1 files changed, 26 insertions(+), 56 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index a4a4497..7038433 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6523,6 +6523,27 @@ abort:
 	return 0;
 }
 
+static void imsm_update_metadata_locally(struct supertype *st,
+				     struct metadata_update *mu)
+{
+	void **space_list;
+
+	if ((!mu) || (!st))
+		return;
+
+	imsm_prepare_update(st, mu);
+	imsm_process_update(st, mu);
+
+	if (mu->space_list) {
+		space_list = (void **)*mu->space_list;
+		while (space_list) {
+			void *space = space_list;
+			space_list = *space_list;
+			free(space);
+		}
+		mu->space_list = NULL;
+	}
+}
 
 static int imsm_reshape_super(struct supertype *st, long long size, int level,
 			      int layout, int chunksize, int raid_disks,
@@ -6565,11 +6586,6 @@ static int imsm_reshape_super(struct supertype *st, long long size, int level,
 			    st, &geo, &old_raid_disks)) {
 			struct imsm_update_reshape *u = NULL;
 			int len;
-			struct intel_super *super = st->sb;
-			void **space_list;
-			struct intel_dev *dl;
-			void **space_tail = (void **)&space_list;
-
 
 			len = imsm_create_metadata_update_for_reshape(
 				st, &geo, old_raid_disks, &u);
@@ -6579,58 +6595,12 @@ static int imsm_reshape_super(struct supertype *st, long long size, int level,
 				goto exit_imsm_reshape_super;
 			}
 
-			/* As well as creating update, we apply update.
-			 */
+			ret_val = 0;
+			append_metadata_update(st, u, len);
 
-			dprintf("imsm:prepare space list for update_reshape\n");
-			for (dl = super->devlist; dl;
-			     dl = dl->next) {
-				int size = sizeof_imsm_dev(dl->dev, 1);
-				void *s;
-				if (u->new_raid_disks > u->old_raid_disks)
-					size += sizeof(__u32)*2*
-					(u->new_raid_disks - u->old_raid_disks);
-				s = malloc(size);
-				if (!s)
-					break;
-				*space_tail = s;
-				space_tail = s;
-				*space_tail = NULL;
-			}
-			ret_val = apply_reshape_container_disks_update(
-				u, super, &space_list);
-			if (ret_val) {
-				/* reallocate anchor
-				 */
-				size_t buf_len = super->len;
-				size_t len =
-					disks_to_mpb_size(u->new_raid_disks);
-				struct imsm_super *mpb = super->anchor;
-				void *new_anchor;
-
-				if (__le32_to_cpu(mpb->mpb_size) + len >
-				    buf_len) {
-					buf_len = ROUND_UP(__le32_to_cpu(
-						mpb->mpb_size) + len, 512);
-					if (posix_memalign(&new_anchor,
-							   512, buf_len) == 0) {
-						memcpy(new_anchor, super->buf,
-						       super->len);
-						free(super->buf);
-						super->buf = new_anchor;
-						super->len = buf_len;
-					}
-					super->updates_pending++;
-					ret_val = 0;
-				}
-			} else {
-				while (space_list) {
-					void *space = space_list;
-					space_list = *space_list;
-					free(space);
-				}
-				free(u);
-			}
+			/* update metadata locally
+			 */
+			imsm_update_metadata_locally(st, st->updates);
 		} else
 			fprintf(stderr, Name "imsm: Operation is not allowed "
 				"on this container\n");

Re: [PATCH 2/8] imsm: FIX: mdadm should process local data

[NeilBrown]

On Wed, 12 Jan 2011 14:54:34 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When update is created by mdadm, local information should be updated also.
> This makes us to prepare one update for mdmon and second "update" to maintain local changes.
> we can use prepared update for "local/mdadm" metadata update purposes.
> We have 2 cases:
> 1. when metadata is updated by mdmon, we avoid metadata reloading in mdadm.
>    we proceed the same updtate 2 times:
> 	- one time in mdadm for "local update"
>         - second time in mdmon for real metadat update
> 2. when metadata is updated by mdadm (no mdmon running) updates are processed in the same
>    way.
> 	- one time in mdadm for "local update"
> 	- there is no "second time" update but mdadm just flushes
>           metadata to array
>    This let us to avoid code duplication by using prepare and process update functions
>    as for update via mdmon. This makes update preparing mdmon independent
>    and there is no need to maintain the same thing in 2 places in code.
> 

I've applied something like this, but with some clean-ups which make the
previous patch unnecessary.

NeilBrown


> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |   82 ++++++++++++++++++---------------------------------------
>  1 files changed, 26 insertions(+), 56 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index a4a4497..7038433 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -6523,6 +6523,27 @@ abort:
>  	return 0;
>  }
>  
> +static void imsm_update_metadata_locally(struct supertype *st,
> +				     struct metadata_update *mu)
> +{
> +	void **space_list;
> +
> +	if ((!mu) || (!st))
> +		return;
> +
> +	imsm_prepare_update(st, mu);
> +	imsm_process_update(st, mu);
> +
> +	if (mu->space_list) {
> +		space_list = (void **)*mu->space_list;
> +		while (space_list) {
> +			void *space = space_list;
> +			space_list = *space_list;
> +			free(space);
> +		}
> +		mu->space_list = NULL;
> +	}
> +}
>  
>  static int imsm_reshape_super(struct supertype *st, long long size, int level,
>  			      int layout, int chunksize, int raid_disks,
> @@ -6565,11 +6586,6 @@ static int imsm_reshape_super(struct supertype *st, long long size, int level,
>  			    st, &geo, &old_raid_disks)) {
>  			struct imsm_update_reshape *u = NULL;
>  			int len;
> -			struct intel_super *super = st->sb;
> -			void **space_list;
> -			struct intel_dev *dl;
> -			void **space_tail = (void **)&space_list;
> -
>  
>  			len = imsm_create_metadata_update_for_reshape(
>  				st, &geo, old_raid_disks, &u);
> @@ -6579,58 +6595,12 @@ static int imsm_reshape_super(struct supertype *st, long long size, int level,
>  				goto exit_imsm_reshape_super;
>  			}
>  
> -			/* As well as creating update, we apply update.
> -			 */
> +			ret_val = 0;
> +			append_metadata_update(st, u, len);
>  
> -			dprintf("imsm:prepare space list for update_reshape\n");
> -			for (dl = super->devlist; dl;
> -			     dl = dl->next) {
> -				int size = sizeof_imsm_dev(dl->dev, 1);
> -				void *s;
> -				if (u->new_raid_disks > u->old_raid_disks)
> -					size += sizeof(__u32)*2*
> -					(u->new_raid_disks - u->old_raid_disks);
> -				s = malloc(size);
> -				if (!s)
> -					break;
> -				*space_tail = s;
> -				space_tail = s;
> -				*space_tail = NULL;
> -			}
> -			ret_val = apply_reshape_container_disks_update(
> -				u, super, &space_list);
> -			if (ret_val) {
> -				/* reallocate anchor
> -				 */
> -				size_t buf_len = super->len;
> -				size_t len =
> -					disks_to_mpb_size(u->new_raid_disks);
> -				struct imsm_super *mpb = super->anchor;
> -				void *new_anchor;
> -
> -				if (__le32_to_cpu(mpb->mpb_size) + len >
> -				    buf_len) {
> -					buf_len = ROUND_UP(__le32_to_cpu(
> -						mpb->mpb_size) + len, 512);
> -					if (posix_memalign(&new_anchor,
> -							   512, buf_len) == 0) {
> -						memcpy(new_anchor, super->buf,
> -						       super->len);
> -						free(super->buf);
> -						super->buf = new_anchor;
> -						super->len = buf_len;
> -					}
> -					super->updates_pending++;
> -					ret_val = 0;
> -				}
> -			} else {
> -				while (space_list) {
> -					void *space = space_list;
> -					space_list = *space_list;
> -					free(space);
> -				}
> -				free(u);
> -			}
> +			/* update metadata locally
> +			 */
> +			imsm_update_metadata_locally(st, st->updates);
>  		} else
>  			fprintf(stderr, Name "imsm: Operation is not allowed "
>  				"on this container\n");
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/8] imsm: FIX: local mdadm update shouldn't be done in update creation function.

[Adam Kwolek]

Local update is performed based on created update, so this code can broke
local update and it is not necessary as prepare and process update functions
are used.

Code removed.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 super-intel.c |   13 -------------
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 7038433..de41702 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6492,19 +6492,6 @@ static int imsm_create_metadata_update_for_reshape(
 		mpb->num_disks++;
 		dev = dev->next;
 	}
-	/* Now update the metadata so that container_content will find
-	 * the new devices
-	 */
-	for (i = 0; i < mpb->num_raid_devs; i++) {
-		int d;
-		struct imsm_dev *dev = get_imsm_dev(super, i);
-		struct imsm_map *map = get_imsm_map(dev, 0);
-		map->num_members = geo->raid_disks;
-		for (d = 0; d < delta_disks; d++) {
-			set_imsm_ord_tbl_ent(map, old_raid_disks + d,
-					     mpb->num_disks - delta_disks + d);
-		}
-	}
 
 abort:
 	/* free spares

Re: [PATCH 3/8] imsm: FIX: local mdadm update shouldn't be done in update creation function.

[NeilBrown]

On Wed, 12 Jan 2011 14:54:42 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Local update is performed based on created update, so this code can broke
> local update and it is not necessary as prepare and process update functions
> are used.
> 
> Code removed.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |   13 -------------
>  1 files changed, 0 insertions(+), 13 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index 7038433..de41702 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -6492,19 +6492,6 @@ static int imsm_create_metadata_update_for_reshape(
>  		mpb->num_disks++;
>  		dev = dev->next;
>  	}
> -	/* Now update the metadata so that container_content will find
> -	 * the new devices
> -	 */
> -	for (i = 0; i < mpb->num_raid_devs; i++) {
> -		int d;
> -		struct imsm_dev *dev = get_imsm_dev(super, i);
> -		struct imsm_map *map = get_imsm_map(dev, 0);
> -		map->num_members = geo->raid_disks;
> -		for (d = 0; d < delta_disks; d++) {
> -			set_imsm_ord_tbl_ent(map, old_raid_disks + d,
> -					     mpb->num_disks - delta_disks + d);
> -		}
> -	}
>  
>  abort:
>  	/* free spares

Applied, thanks.

NeilBrown

[PATCH 4/8] imsm: FIX: old devices memory has to be released

[Adam Kwolek]

When process_update() replaces memory for bigger devices, old memory areas are collected in a list
and has to be assigned in to pointer in update for later release.

List created from old devices is created and attached to space_list for later releasing.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 super-intel.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index de41702..c3ff365 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -5744,6 +5744,8 @@ static int apply_reshape_container_disks_update(struct imsm_update_reshape *u,
 		*sp = tofree;
 		tofree = sp;
 	}
+	if (tofree)
+		*space_list = tofree;
 	ret_val = 1;
 
 update_reshape_exit:

Re: [PATCH 4/8] imsm: FIX: old devices memory has to be released

[NeilBrown]

On Wed, 12 Jan 2011 14:54:50 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When process_update() replaces memory for bigger devices, old memory areas are collected in a list
> and has to be assigned in to pointer in update for later release.
> 
> List created from old devices is created and attached to space_list for later releasing.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index de41702..c3ff365 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -5744,6 +5744,8 @@ static int apply_reshape_container_disks_update(struct imsm_update_reshape *u,
>  		*sp = tofree;
>  		tofree = sp;
>  	}
> +	if (tofree)
> +		*space_list = tofree;
>  	ret_val = 1;
>  
>  update_reshape_exit:


Applied, thanks,

NeilBrown

[PATCH 5/8] FIX: Cannot load container information

[Adam Kwolek]

When container is passed to grow_reshape(), load_container() function has to be used to
get all required information from metadata. In such case subarray variable is not set.
This means that currently used condition is wrong.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 Grow.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index c2e84f3..519fbc2 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1356,7 +1356,7 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
 		fmt_devname(container_buf, container_dev);
 		container = container_buf;
 
-		if (subarray)
+		if (!subarray)
 			rv = st->ss->load_container(st, cfd, NULL);
 		else
 			rv = st->ss->load_super(st, cfd, NULL);

Re: [PATCH 5/8] FIX: Cannot load container information

[NeilBrown]

On Wed, 12 Jan 2011 14:55:03 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When container is passed to grow_reshape(), load_container() function has to be used to
> get all required information from metadata. In such case subarray variable is not set.
> This means that currently used condition is wrong.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index c2e84f3..519fbc2 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -1356,7 +1356,7 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
>  		fmt_devname(container_buf, container_dev);
>  		container = container_buf;
>  
> -		if (subarray)
> +		if (!subarray)
>  			rv = st->ss->load_container(st, cfd, NULL);
>  		else
>  			rv = st->ss->load_super(st, cfd, NULL);

load_super is never correct here, so I change this to always call
load_container.

Thanks,
NeilBrown

[PATCH 6/8] imsm: FIX: spares are not counted

[Adam Kwolek]

Field info->array.spare_disks is used on begin of reshape_array() to check
if there is enough number of spares to process reshape.
During container_content_imsm() call spare disks are not counted.
This causes that reshape_array() reports that there is not enough spares
to execute reshape.

Patch adds spares counting for reshape process.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 super-intel.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index c3ff365..0c70fda 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -4632,9 +4632,13 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
 			if (map2) {
 				if (slot < map2->num_members)
 					info_d->disk.state = (1 << MD_DISK_ACTIVE);
+				else
+					this->array.spare_disks++;
 			} else {
 				if (slot < map->num_members)
 					info_d->disk.state = (1 << MD_DISK_ACTIVE);
+				else
+					this->array.spare_disks++;
 			}
 			if (info_d->recovery_start == MaxSector)
 				this->array.working_disks++;

Re: [PATCH 6/8] imsm: FIX: spares are not counted

[NeilBrown]

On Wed, 12 Jan 2011 14:55:10 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> Field info->array.spare_disks is used on begin of reshape_array() to check
> if there is enough number of spares to process reshape.
> During container_content_imsm() call spare disks are not counted.
> This causes that reshape_array() reports that there is not enough spares
> to execute reshape.
> 
> Patch adds spares counting for reshape process.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  super-intel.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/super-intel.c b/super-intel.c
> index c3ff365..0c70fda 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -4632,9 +4632,13 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
>  			if (map2) {
>  				if (slot < map2->num_members)
>  					info_d->disk.state = (1 << MD_DISK_ACTIVE);
> +				else
> +					this->array.spare_disks++;
>  			} else {
>  				if (slot < map->num_members)
>  					info_d->disk.state = (1 << MD_DISK_ACTIVE);
> +				else
> +					this->array.spare_disks++;
>  			}
>  			if (info_d->recovery_start == MaxSector)
>  				this->array.working_disks++;


Applied, thanks.

NeilBrown

[PATCH 7/8] Finalize reshape after adding disks to array

[Adam Kwolek]

When reshape is finished, monitor has to finalize reshape in metadata for curent array.
To do this set_array_state() should be called.
This finishes migration and stores metadata on disks.

This finishes reshape flow in mdmon.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 monitor.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/monitor.c b/monitor.c
index 1107d47..a0068d3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -322,6 +322,17 @@ static int read_and_act(struct active_array *a)
 		 */
 		check_reshape = 1;
 
+	/* finalize reshape detection
+	 */
+	if ((a->curr_action != reshape) &&
+	    (a->prev_action == reshape)) {
+		/* A reshape has finished.
+		 * Some disks may be in sync now.
+		 */
+		a->container->ss->set_array_state(a, a->curr_state <= clean);
+		check_degraded = 1;
+	}
+
 	/* Check for failures and if found:
 	 * 1/ Record the failure in the metadata and unblock the device.
 	 *    FIXME update the kernel to stop notifying on failed drives when

Re: [PATCH 7/8] Finalize reshape after adding disks to array

[NeilBrown]

On Wed, 12 Jan 2011 14:55:18 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When reshape is finished, monitor has to finalize reshape in metadata for curent array.
> To do this set_array_state() should be called.
> This finishes migration and stores metadata on disks.
> 
> This finishes reshape flow in mdmon.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  monitor.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 1107d47..a0068d3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -322,6 +322,17 @@ static int read_and_act(struct active_array *a)
>  		 */
>  		check_reshape = 1;
>  
> +	/* finalize reshape detection
> +	 */
> +	if ((a->curr_action != reshape) &&
> +	    (a->prev_action == reshape)) {
> +		/* A reshape has finished.
> +		 * Some disks may be in sync now.
> +		 */
> +		a->container->ss->set_array_state(a, a->curr_state <= clean);
> +		check_degraded = 1;
> +	}
> +
>  	/* Check for failures and if found:
>  	 * 1/ Record the failure in the metadata and unblock the device.
>  	 *    FIXME update the kernel to stop notifying on failed drives when
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


One day you will explain to me why the current code isn't sufficient like I
keep asking you do.  Then maybe I'll apply this patch.

Until then, I won't.

NeilBrown

RE: [PATCH 7/8] Finalize reshape after adding disks to array

[Kwolek, Adam]

> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> owner@vger.kernel.org] On Behalf Of NeilBrown
> Sent: Thursday, January 13, 2011 3:43 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 7/8] Finalize reshape after adding disks to array
> 
> On Wed, 12 Jan 2011 14:55:18 +0100 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > When reshape is finished, monitor has to finalize reshape in metadata
> for curent array.
> > To do this set_array_state() should be called.
> > This finishes migration and stores metadata on disks.
> >
> > This finishes reshape flow in mdmon.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> >  monitor.c |   11 +++++++++++
> >  1 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 1107d47..a0068d3 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -322,6 +322,17 @@ static int read_and_act(struct active_array *a)
> >  		 */
> >  		check_reshape = 1;
> >
> > +	/* finalize reshape detection
> > +	 */
> > +	if ((a->curr_action != reshape) &&
> > +	    (a->prev_action == reshape)) {
> > +		/* A reshape has finished.
> > +		 * Some disks may be in sync now.
> > +		 */
> > +		a->container->ss->set_array_state(a, a->curr_state <=
> clean);
> > +		check_degraded = 1;
> > +	}
> > +
> >  	/* Check for failures and if found:
> >  	 * 1/ Record the failure in the metadata and unblock the device.
> >  	 *    FIXME update the kernel to stop notifying on failed drives
> when
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-raid"
> in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> One day you will explain to me why the current code isn't sufficient
> like I
> keep asking you do.  Then maybe I'll apply this patch.
> 
> Until then, I won't.
> 
> NeilBrown

I've made some tests and you are right, current code is enough.

Thanks
Adam

> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 8/8] FIX: reload metadata for container operation

[Adam Kwolek]

When reshape is started metadata tells mdadm that reshape is active via reshape_active variable.
When reshape is finished mdmon updates metadata to end reshape in metadata for current array.
We have reload metadata to get this flag updated and switch to next array (if exists)
for container reshape case. This is indicated by forked flag. Function has to returned
to caller with reloaded metadata information /up to date/.

Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---

 Grow.c |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/Grow.c b/Grow.c
index 519fbc2..3455115 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2021,8 +2021,25 @@ static int reshape_array(char *container, int fd, char *devname,
 					"to %s\n", devname, c);
 		}
 	out:
-		if (forked)
+		if (forked) {
+			if (!rv) {
+				if (container)
+					ping_monitor(container);
+				if (st->ss->external) {
+					/* Re-load the metadata as much
+				 * could have changed
+				 */
+					int cfd = open_dev(st->container_dev);
+					if (cfd >= 0) {
+						st->ss->free_super(st);
+						st->ss->load_container(st, cfd,
+								     container);
+						close(cfd);
+					}
+				}
+			}
 			return 0;
+		}
 		exit(0);
 	case -1:
 		fprintf(stderr, Name ": Cannot run child to monitor reshape: %s\n",

Re: [PATCH 8/8] FIX: reload metadata for container operation

[NeilBrown]

On Wed, 12 Jan 2011 14:55:26 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:

> When reshape is started metadata tells mdadm that reshape is active via reshape_active variable.
> When reshape is finished mdmon updates metadata to end reshape in metadata for current array.
> We have reload metadata to get this flag updated and switch to next array (if exists)
> for container reshape case. This is indicated by forked flag. Function has to returned
> to caller with reloaded metadata information /up to date/.
> 
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
> 
>  Grow.c |   19 ++++++++++++++++++-
>  1 files changed, 18 insertions(+), 1 deletions(-)
> 
> diff --git a/Grow.c b/Grow.c
> index 519fbc2..3455115 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2021,8 +2021,25 @@ static int reshape_array(char *container, int fd, char *devname,
>  					"to %s\n", devname, c);
>  		}
>  	out:
> -		if (forked)
> +		if (forked) {
> +			if (!rv) {
> +				if (container)
> +					ping_monitor(container);
> +				if (st->ss->external) {
> +					/* Re-load the metadata as much
> +				 * could have changed
> +				 */
> +					int cfd = open_dev(st->container_dev);
> +					if (cfd >= 0) {
> +						st->ss->free_super(st);
> +						st->ss->load_container(st, cfd,
> +								     container);
> +						close(cfd);
> +					}
> +				}
> +			}
>  			return 0;
> +		}
>  		exit(0);
>  	case -1:
>  		fprintf(stderr, Name ": Cannot run child to monitor reshape: %s\n",
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Thanks.  Instead of applying this I have rearrange the tail end of
reshape_array substantially which will hopefully have the same effect - or
better.

Thanks,
NeilBrown