[PATCH] imsm: FIX: Sometimes reshape cannot be finished

[PATCH] imsm: FIX: Sometimes reshape cannot be finished

am 14.06.2011 14:56:22 von adam.kwolek

When array size is not aligned to copy area, number of migration unit
is increased in init_migr_record_imsm():7665 to reshape whole array.
During calculation of last migration unit, this should be in mind also,
otherwise checkpoint (max-1) is always written and reshape
is never finished in mdadm.

Signed-off-by: Adam Kwolek
---

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

diff --git a/super-intel.c b/super-intel.c
index 5dc8ca8..d525295 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7788,20 +7788,30 @@ abort:
int save_checkpoint_imsm(struct supertype *st, struct mdinfo *info, int state)
{
struct intel_super *super = st->sb;
+ unsigned long long blocks_per_unit;
+
if (load_imsm_migr_rec(super, info) != 0) {
dprintf("imsm: ERROR: Cannot read migration record "
"for checkpoint save.\n");
return 1;
}

- if (__le32_to_cpu(super->migr_rec->blocks_per_unit) == 0) {
+ blocks_per_unit = __le32_to_cpu(super->migr_rec->blocks_per_unit);
+ if (blocks_per_unit == 0) {
dprintf("imsm: no migration in progress.\n");
return 2;
}
-
super->migr_rec->curr_migr_unit =
- __cpu_to_le32(info->reshape_progress /
- __le32_to_cpu(super->migr_rec->blocks_per_unit));
+ info->reshape_progress / blocks_per_unit;
+ /* check if array is alligned to copy area
+ * if it is not alligned, add one to current migration unit value
+ * this can happend on array reshape finish only
+ */
+ if (info->reshape_progress % blocks_per_unit)
+ super->migr_rec->curr_migr_unit++;
+ super->migr_rec->curr_migr_unit =
+ __cpu_to_le32(super->migr_rec->curr_migr_unit);
+
super->migr_rec->rec_status = __cpu_to_le32(state);
super->migr_rec->dest_1st_member_lba =
__cpu_to_le32((__le32_to_cpu(super->migr_rec->curr_migr_unit ))

--
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] imsm: FIX: Sometimes reshape cannot be finished

am 15.06.2011 01:15:36 von NeilBrown

On Tue, 14 Jun 2011 14:56:22 +0200 Adam Kwolek wrote:

> When array size is not aligned to copy area, number of migration unit
> is increased in init_migr_record_imsm():7665 to reshape whole array.
> During calculation of last migration unit, this should be in mind also,
> otherwise checkpoint (max-1) is always written and reshape
> is never finished in mdadm.
>
> Signed-off-by: Adam Kwolek
> ---
>
> super-intel.c | 18 ++++++++++++++----
> 1 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index 5dc8ca8..d525295 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -7788,20 +7788,30 @@ abort:
> int save_checkpoint_imsm(struct supertype *st, struct mdinfo *info, int state)
> {
> struct intel_super *super = st->sb;
> + unsigned long long blocks_per_unit;
> +
> if (load_imsm_migr_rec(super, info) != 0) {
> dprintf("imsm: ERROR: Cannot read migration record "
> "for checkpoint save.\n");
> return 1;
> }
>
> - if (__le32_to_cpu(super->migr_rec->blocks_per_unit) == 0) {
> + blocks_per_unit = __le32_to_cpu(super->migr_rec->blocks_per_unit);
> + if (blocks_per_unit == 0) {
> dprintf("imsm: no migration in progress.\n");
> return 2;
> }
> -
> super->migr_rec->curr_migr_unit =
> - __cpu_to_le32(info->reshape_progress /
> - __le32_to_cpu(super->migr_rec->blocks_per_unit));
> + info->reshape_progress / blocks_per_unit;
> + /* check if array is alligned to copy area
> + * if it is not alligned, add one to current migration unit value
> + * this can happend on array reshape finish only
> + */
> + if (info->reshape_progress % blocks_per_unit)
> + super->migr_rec->curr_migr_unit++;
> + super->migr_rec->curr_migr_unit =
> + __cpu_to_le32(super->migr_rec->curr_migr_unit);
> +
> super->migr_rec->rec_status = __cpu_to_le32(state);
> super->migr_rec->dest_1st_member_lba =
> __cpu_to_le32((__le32_to_cpu(super->migr_rec->curr_migr_unit ))


I think it is a bit untidy storing a value in cpu-order in super->migr_rec
and then changing it to little-endian later. It feels error prone.
So I have changed it to use a local variable for the cpu-order version as
below.

Thanks,
NeilBrown

commit f8b72ef517ea305f714230b0503f94ae5f1b8fa4
Author: Adam Kwolek
Date: Wed Jun 15 09:13:49 2011 +1000

imsm: FIX: Sometimes reshape cannot be finished

When array size is not aligned to copy area, number of migration unit
is increased in init_migr_record_imsm():7665 to reshape whole array.
During calculation of last migration unit, this should be in mind also,
otherwise checkpoint (max-1) is always written and reshape
is never finished in mdadm.

Signed-off-by: Adam Kwolek
Signed-off-by: NeilBrown

diff --git a/super-intel.c b/super-intel.c
index 5dc8ca8..bb76f74 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7788,24 +7788,34 @@ abort:
int save_checkpoint_imsm(struct supertype *st, struct mdinfo *info, int state)
{
struct intel_super *super = st->sb;
+ unsigned long long blocks_per_unit;
+ unsigned long long curr_migr_unit;
+
if (load_imsm_migr_rec(super, info) != 0) {
dprintf("imsm: ERROR: Cannot read migration record "
"for checkpoint save.\n");
return 1;
}

- if (__le32_to_cpu(super->migr_rec->blocks_per_unit) == 0) {
+ blocks_per_unit = __le32_to_cpu(super->migr_rec->blocks_per_unit);
+ if (blocks_per_unit == 0) {
dprintf("imsm: no migration in progress.\n");
return 2;
}
+ curr_migr_unit = info->reshape_progress / blocks_per_unit;
+ /* check if array is alligned to copy area
+ * if it is not alligned, add one to current migration unit value
+ * this can happend on array reshape finish only
+ */
+ if (info->reshape_progress % blocks_per_unit)
+ curr_migr_unit++;

super->migr_rec->curr_migr_unit =
- __cpu_to_le32(info->reshape_progress /
- __le32_to_cpu(super->migr_rec->blocks_per_unit));
+ __cpu_to_le32(curr_migr_unit);
super->migr_rec->rec_status = __cpu_to_le32(state);
super->migr_rec->dest_1st_member_lba =
- __cpu_to_le32((__le32_to_cpu(super->migr_rec->curr_migr_unit ))
- * __le32_to_cpu(super->migr_rec->dest_depth_per_unit));
+ __cpu_to_le32(curr_migr_unit *
+ __le32_to_cpu(super->migr_rec->dest_depth_per_unit));
if (write_imsm_migr_rec(st) < 0) {
dprintf("imsm: Cannot write migration record "
"outside backup area\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