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

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
--
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 1/8] FIX: put update in to queue for local meta update

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
---

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 */


--
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 2/8] imsm: FIX: mdadm should process local data

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
---

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

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
---

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

--
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 4/8] imsm: FIX: old devices memory has to be released

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
---

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:

--
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 5/8] FIX: Cannot load container information

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
---

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);

--
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 6/8] imsm: FIX: spares are not counted

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
---

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++;

--
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 7/8] Finalize reshape after adding disks to array

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
---

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

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

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
---

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

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

On Wed, 12 Jan 2011 14:54:34 +0100 Adam Kwolek 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
> ---
>
> 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

--
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

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

On Wed, 12 Jan 2011 14:54:42 +0100 Adam Kwolek 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
> ---
>
> 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

--
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

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

On Wed, 12 Jan 2011 14:54:50 +0100 Adam Kwolek 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
> ---
>
> 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

--
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

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

On Wed, 12 Jan 2011 14:55:03 +0100 Adam Kwolek 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
> ---
>
> 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

--
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

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

On Wed, 12 Jan 2011 14:55:10 +0100 Adam Kwolek 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
> ---
>
> 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

--
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

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

On Wed, 12 Jan 2011 14:55:18 +0100 Adam Kwolek 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
> ---
>
> 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

--
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

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

On Wed, 12 Jan 2011 14:55:26 +0100 Adam Kwolek 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
> ---
>
> 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

--
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

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

> -----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
> 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
> > ---
> >
> > 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
--
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