* [PATCH 01/21] imsm: FIX: Cannot create volume
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
@ 2011-06-08 16:09 ` Adam Kwolek
2011-06-09 2:42 ` NeilBrown
2011-06-08 16:09 ` [PATCH 02/21] imsm: FIX: Opened handle is not closed Adam Kwolek
` (20 subsequent siblings)
21 siblings, 1 reply; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:09 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Clearing info structure causes mdadm is not able to create workable volume.
During volume creation info structure passed to getinfo() function
contains some information already and cannot be cleared.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index b8d8b4e..471dbd2 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2075,7 +2075,6 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
unsigned int component_size_alligment;
int map_disks = info->array.raid_disks;
- memset(info, 0, sizeof(*info));
if (prev_map)
map_to_analyse = prev_map;
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 01/21] imsm: FIX: Cannot create volume
2011-06-08 16:09 ` [PATCH 01/21] imsm: FIX: Cannot create volume Adam Kwolek
@ 2011-06-09 2:42 ` NeilBrown
2011-06-09 6:40 ` Kwolek, Adam
0 siblings, 1 reply; 25+ messages in thread
From: NeilBrown @ 2011-06-09 2:42 UTC (permalink / raw)
To: Adam Kwolek
Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
On Wed, 08 Jun 2011 18:09:41 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:
> Clearing info structure causes mdadm is not able to create workable volume.
>
> During volume creation info structure passed to getinfo() function
> contains some information already and cannot be cleared.
>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
>
> super-intel.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/super-intel.c b/super-intel.c
> index b8d8b4e..471dbd2 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -2075,7 +2075,6 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
> unsigned int component_size_alligment;
> int map_disks = info->array.raid_disks;
>
> - memset(info, 0, sizeof(*info));
> if (prev_map)
> map_to_analyse = prev_map;
>
>
> --
> 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
I'm sorry, but this is just a very silly patch.
In many caches the 'info' structure is completely uninitialised, so it really
does make sense to initialise it, which is why I added that.
Just removing it *must* be wrong.
It would be much more helpful if you had explained what the "some
information" was.
Maybe it is the '->next' field that container_content_imsm() sets before
calling getinfo_super_imsm_volume()?? Well that getinfo_super_imsm_volume
doesn't use that field, so we can just move the assignment afterwards.
or maybe ... getinfo_super_imsm_volume uses info->disk.raid_disk, but no
caller ever sets that up that I can see, so that code is plainly wrong
(though ddf makes the same mistake so that is probably my fault).
I've 'fixed' them both to just report on the 'first' disk as that is no worse
than what they currently do.
I that doesn't fix your problem, please explain exactly what is being cleared
that shouldn't be.
NeilBrown
commit 9894ec0d64a9faab719d016bbbf5fbc842757df6
Author: NeilBrown <neilb@suse.de>
Date: Thu Jun 9 12:42:02 2011 +1000
Fix some fall-out from recent memset-zero for getinfo_super
container_content_imsm was setting info->next before calling
getinfo_super_imsm_container which now zeros everything.
So move that assignment to afterwards.
So both imsm and ddf were assuming info->disk.raid_disk means
something but it doesn't. So fix those.
Signed-off-by: NeilBrown <neilb@suse.de>
diff --git a/super-ddf.c b/super-ddf.c
index 21a917e..3fba2eb 100644
--- a/super-ddf.c
+++ b/super-ddf.c
@@ -1429,9 +1429,7 @@ static void getinfo_super_ddf_bvd(struct supertype *st, struct mdinfo *info, cha
info->component_size = __be64_to_cpu(vc->conf.blocks);
}
- for (dl = ddf->dlist; dl ; dl = dl->next)
- if (dl->raiddisk == info->disk.raid_disk)
- break;
+ dl = ddf->dlist;
info->disk.major = 0;
info->disk.minor = 0;
if (dl) {
diff --git a/super-intel.c b/super-intel.c
index b8d8b4e..6fed9eb 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2079,9 +2079,8 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
if (prev_map)
map_to_analyse = prev_map;
- for (dl = super->disks; dl; dl = dl->next)
- if (dl->raiddisk == info->disk.raid_disk)
- break;
+ dl = super->disks;
+
info->container_member = super->current_vol;
info->array.raid_disks = map->num_members;
info->array.level = get_imsm_raid_level(map_to_analyse);
@@ -5446,11 +5445,10 @@ static struct mdinfo *container_content_imsm(struct supertype *st, char *subarra
sizeof(*this));
break;
}
- memset(this, 0, sizeof(*this));
- this->next = rest;
super->current_vol = i;
getinfo_super_imsm_volume(st, this, NULL);
+ this->next = rest;
for (slot = 0 ; slot < map->num_members; slot++) {
unsigned long long recovery_start;
struct mdinfo *info_d;
^ permalink raw reply related [flat|nested] 25+ messages in thread* RE: [PATCH 01/21] imsm: FIX: Cannot create volume
2011-06-09 2:42 ` NeilBrown
@ 2011-06-09 6:40 ` Kwolek, Adam
0 siblings, 0 replies; 25+ messages in thread
From: Kwolek, Adam @ 2011-06-09 6:40 UTC (permalink / raw)
To: NeilBrown
Cc: linux-raid@vger.kernel.org, Williams, Dan J, Ciechanowski, Ed,
Neubauer, Wojciech
> -----Original Message-----
> From: NeilBrown [mailto:neilb@suse.de]
> Sent: Thursday, June 09, 2011 4:42 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH 01/21] imsm: FIX: Cannot create volume
>
> On Wed, 08 Jun 2011 18:09:41 +0200 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
>
> > Clearing info structure causes mdadm is not able to create workable
> volume.
> >
> > During volume creation info structure passed to getinfo() function
> > contains some information already and cannot be cleared.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> > super-intel.c | 1 -
> > 1 files changed, 0 insertions(+), 1 deletions(-)
> >
> > diff --git a/super-intel.c b/super-intel.c
> > index b8d8b4e..471dbd2 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -2075,7 +2075,6 @@ static void getinfo_super_imsm_volume(struct
> supertype *st, struct mdinfo *info,
> > unsigned int component_size_alligment;
> > int map_disks = info->array.raid_disks;
> >
> > - memset(info, 0, sizeof(*info));
> > if (prev_map)
> > map_to_analyse = prev_map;
> >
> >
> > --
> > 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
>
>
> I'm sorry, but this is just a very silly patch.
You are right in your suspicions about root cause. It is link problem using 'next' field.
(I can see this problem at this moment)
I'll review it and I'll make it less silly ;)
BR
Adam
>
> In many caches the 'info' structure is completely uninitialised, so it
> really
> does make sense to initialise it, which is why I added that.
> Just removing it *must* be wrong.
>
> It would be much more helpful if you had explained what the "some
> information" was.
>
> Maybe it is the '->next' field that container_content_imsm() sets before
> calling getinfo_super_imsm_volume()?? Well that
> getinfo_super_imsm_volume
> doesn't use that field, so we can just move the assignment afterwards.
>
> or maybe ... getinfo_super_imsm_volume uses info->disk.raid_disk, but no
> caller ever sets that up that I can see, so that code is plainly wrong
> (though ddf makes the same mistake so that is probably my fault).
> I've 'fixed' them both to just report on the 'first' disk as that is no
> worse
> than what they currently do.
>
> I that doesn't fix your problem, please explain exactly what is being
> cleared
> that shouldn't be.
>
> NeilBrown
>
> commit 9894ec0d64a9faab719d016bbbf5fbc842757df6
> Author: NeilBrown <neilb@suse.de>
> Date: Thu Jun 9 12:42:02 2011 +1000
>
> Fix some fall-out from recent memset-zero for getinfo_super
>
> container_content_imsm was setting info->next before calling
> getinfo_super_imsm_container which now zeros everything.
> So move that assignment to afterwards.
>
> So both imsm and ddf were assuming info->disk.raid_disk means
> something but it doesn't. So fix those.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
>
> diff --git a/super-ddf.c b/super-ddf.c
> index 21a917e..3fba2eb 100644
> --- a/super-ddf.c
> +++ b/super-ddf.c
> @@ -1429,9 +1429,7 @@ static void getinfo_super_ddf_bvd(struct supertype
> *st, struct mdinfo *info, cha
> info->component_size = __be64_to_cpu(vc->conf.blocks);
> }
>
> - for (dl = ddf->dlist; dl ; dl = dl->next)
> - if (dl->raiddisk == info->disk.raid_disk)
> - break;
> + dl = ddf->dlist;
> info->disk.major = 0;
> info->disk.minor = 0;
> if (dl) {
> diff --git a/super-intel.c b/super-intel.c
> index b8d8b4e..6fed9eb 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -2079,9 +2079,8 @@ static void getinfo_super_imsm_volume(struct
> supertype *st, struct mdinfo *info,
> if (prev_map)
> map_to_analyse = prev_map;
>
> - for (dl = super->disks; dl; dl = dl->next)
> - if (dl->raiddisk == info->disk.raid_disk)
> - break;
> + dl = super->disks;
> +
> info->container_member = super->current_vol;
> info->array.raid_disks = map->num_members;
> info->array.level = get_imsm_raid_level(map_to_analyse);
> @@ -5446,11 +5445,10 @@ static struct mdinfo
> *container_content_imsm(struct supertype *st, char *subarra
> sizeof(*this));
> break;
> }
> - memset(this, 0, sizeof(*this));
> - this->next = rest;
>
> super->current_vol = i;
> getinfo_super_imsm_volume(st, this, NULL);
> + this->next = rest;
> for (slot = 0 ; slot < map->num_members; slot++) {
> unsigned long long recovery_start;
> struct mdinfo *info_d;
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 02/21] imsm: FIX: Opened handle is not closed
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
2011-06-08 16:09 ` [PATCH 01/21] imsm: FIX: Cannot create volume Adam Kwolek
@ 2011-06-08 16:09 ` Adam Kwolek
2011-06-08 16:09 ` [PATCH 03/21] imsm: FIX: Verify if migration record is loaded correctly Adam Kwolek
` (19 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:09 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Opened file handle should be closed before function exit.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 471dbd2..01ffcc8 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8550,8 +8550,10 @@ int wait_for_reshape_imsm(struct mdinfo *sra, unsigned long long to_complete,
sysfs_set_str(sra, NULL, "sync_max", "max");
to_complete = MaxSector;
} else {
- if (completed > to_complete)
+ if (completed > to_complete) {
+ close(fd);
return -1;
+ }
if (sysfs_set_num(sra, NULL, "sync_max",
to_complete / ndata) != 0) {
close(fd);
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 03/21] imsm: FIX: Verify if migration record is loaded correctly
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
2011-06-08 16:09 ` [PATCH 01/21] imsm: FIX: Cannot create volume Adam Kwolek
2011-06-08 16:09 ` [PATCH 02/21] imsm: FIX: Opened handle is not closed Adam Kwolek
@ 2011-06-08 16:09 ` Adam Kwolek
2011-06-08 16:10 ` [PATCH 04/21] imsm: FIX: Detect migration end during migration record saving Adam Kwolek
` (18 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:09 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Migration compatibility can be checked when general migration record
is present.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 32 +++++++++++++++++++-------------
1 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 01ffcc8..269cb0a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -3038,8 +3038,8 @@ static int load_imsm_mpb(int fd, struct intel_super *super, char *devname)
if (lseek64(fd, dsize - (512 * 2), SEEK_SET) < 0) {
if (devname)
- fprintf(stderr,
- Name ": Cannot seek to anchor block on %s: %s\n",
+ fprintf(stderr, Name
+ ": Cannot seek to anchor block on %s: %s\n",
devname, strerror(errno));
return 1;
}
@@ -3836,16 +3836,17 @@ static int load_super_imsm(struct supertype *st, int fd, char *devname)
}
/* load migration record */
- load_imsm_migr_rec(super, NULL);
-
- /* Check for unsupported migration features */
- if (check_mpb_migr_compatibility(super) != 0) {
- fprintf(stderr, Name ": Unsupported migration detected");
- if (devname)
- fprintf(stderr, " on %s\n", devname);
- else
- fprintf(stderr, " (IMSM).\n");
- return 3;
+ if (load_imsm_migr_rec(super, NULL) == 0) {
+ /* Check for unsupported migration features */
+ if (check_mpb_migr_compatibility(super) != 0) {
+ fprintf(stderr,
+ Name ": Unsupported migration detected");
+ if (devname)
+ fprintf(stderr, " on %s\n", devname);
+ else
+ fprintf(stderr, " (IMSM).\n");
+ return 3;
+ }
}
return 0;
@@ -7763,7 +7764,12 @@ abort:
int save_checkpoint_imsm(struct supertype *st, struct mdinfo *info, int state)
{
struct intel_super *super = st->sb;
- load_imsm_migr_rec(super, info);
+ 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) {
dprintf("ERROR: blocks_per_unit = 0!!!\n");
return 1;
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 04/21] imsm: FIX: Detect migration end during migration record saving
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (2 preceding siblings ...)
2011-06-08 16:09 ` [PATCH 03/21] imsm: FIX: Verify if migration record is loaded correctly Adam Kwolek
@ 2011-06-08 16:10 ` Adam Kwolek
2011-06-08 16:10 ` [PATCH 05/21] imsm: FIX: Max position could not be rounded to MB Adam Kwolek
` (17 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:10 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Checkpoint should be saved when migration is in progress only.
End of reshape (based on passes status) should be detected and it should
not cause abort of reshape/check-pointing/ operation.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 269cb0a..3afa913 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7760,6 +7760,8 @@ abort:
* Returns:
* 0: success
* 1: failure
+ * 2: failure, means no valid migration record
+ * / no general migration in progress /
******************************************************************************/
int save_checkpoint_imsm(struct supertype *st, struct mdinfo *info, int state)
{
@@ -7771,8 +7773,8 @@ int save_checkpoint_imsm(struct supertype *st, struct mdinfo *info, int state)
}
if (__le32_to_cpu(super->migr_rec->blocks_per_unit) == 0) {
- dprintf("ERROR: blocks_per_unit = 0!!!\n");
- return 1;
+ dprintf("imsm: no migration in progress.\n");
+ return 2;
}
super->migr_rec->curr_migr_unit =
@@ -8842,7 +8844,9 @@ static int imsm_manage_reshape(
sra->reshape_progress = next_step;
- if (save_checkpoint_imsm(st, sra, UNIT_SRC_NORMAL)) {
+ if (save_checkpoint_imsm(st, sra, UNIT_SRC_NORMAL) == 1) {
+ /* ignore error == 2, this can mean end of reshape here
+ */
dprintf("imsm: Cannot write checkpoint to "
"migration record (UNIT_SRC_NORMAL)\n");
goto abort;
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 05/21] imsm: FIX: Max position could not be rounded to MB
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (3 preceding siblings ...)
2011-06-08 16:10 ` [PATCH 04/21] imsm: FIX: Detect migration end during migration record saving Adam Kwolek
@ 2011-06-08 16:10 ` Adam Kwolek
2011-06-08 16:10 ` [PATCH 06/21] imsm: FIX: Check layout for level migration Adam Kwolek
` (16 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:10 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
When rounded array size information from metadata is used for number
of migration units calculation it can occurs that result of units
can be smaller (-1) than required due to used (rounded) array size).
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 12 +++---------
1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 3afa913..2468968 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7621,10 +7621,7 @@ void init_migr_record_imsm(struct supertype *st, struct imsm_dev *dev,
struct imsm_map *map_dest = get_imsm_map(dev, 0);
struct imsm_map *map_src = get_imsm_map(dev, 1);
unsigned long long num_migr_units;
-
- unsigned long long array_blocks =
- (((unsigned long long)__le32_to_cpu(dev->size_high)) << 32) +
- __le32_to_cpu(dev->size_low);
+ unsigned long long array_blocks;
memset(migr_rec, 0, sizeof(struct migr_record));
migr_rec->family_num = __cpu_to_le32(super->anchor->family_num);
@@ -7640,7 +7637,7 @@ void init_migr_record_imsm(struct supertype *st, struct imsm_dev *dev,
__cpu_to_le32(migr_rec->dest_depth_per_unit * new_data_disks);
migr_rec->dest_depth_per_unit =
__cpu_to_le32(migr_rec->dest_depth_per_unit);
-
+ array_blocks = info->component_size * new_data_disks;
num_migr_units =
array_blocks / __le32_to_cpu(migr_rec->blocks_per_unit);
@@ -8737,10 +8734,7 @@ static int imsm_manage_reshape(
goto abort;
}
- max_position =
- __le32_to_cpu(migr_rec->post_migr_vol_cap) +
- ((unsigned long long)__le32_to_cpu(
- migr_rec->post_migr_vol_cap_hi) << 32);
+ max_position = sra->component_size * ndata;
while (__le32_to_cpu(migr_rec->curr_migr_unit) <
__le32_to_cpu(migr_rec->num_migr_units)) {
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 06/21] imsm: FIX: Check layout for level migration
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (4 preceding siblings ...)
2011-06-08 16:10 ` [PATCH 05/21] imsm: FIX: Max position could not be rounded to MB Adam Kwolek
@ 2011-06-08 16:10 ` Adam Kwolek
2011-06-08 16:10 ` [PATCH 07/21] imsm: FIX: Use macros to data access Adam Kwolek
` (15 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:10 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
When user doesn't specify raid 5 layout for raid0->rai5 migration,
layout structure member is uninitialized. Earlier it cannot be determined
if it is correct or not.
In metadata handle proper verification is placed.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
---
super-intel.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 2468968..3baea6a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8278,7 +8278,6 @@ enum imsm_reshape_type imsm_analyze_change(struct supertype *st,
int chunk;
getinfo_super_imsm_volume(st, &info, NULL);
-
if ((geo->level != info.array.level) &&
(geo->level >= 0) &&
(geo->level != UnSet)) {
@@ -8286,6 +8285,14 @@ enum imsm_reshape_type imsm_analyze_change(struct supertype *st,
case 0:
if (geo->level == 5) {
change = CH_MIGRATION;
+ if (geo->layout != ALGORITHM_LEFT_ASYMMETRIC) {
+ fprintf(stderr,
+ Name " Error. Requested Layout "
+ "not supported (left-asymmetric layout "
+ "is supported only)!\n");
+ change = -1;
+ goto analyse_change_exit;
+ }
check_devs = 1;
}
if (geo->level == 10) {
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 07/21] imsm: FIX: Use macros to data access
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (5 preceding siblings ...)
2011-06-08 16:10 ` [PATCH 06/21] imsm: FIX: Check layout for level migration Adam Kwolek
@ 2011-06-08 16:10 ` Adam Kwolek
2011-06-08 16:10 ` [PATCH 08/21] imsm: FIX: Calculate backup location based on metadata information Adam Kwolek
` (14 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:10 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Metadata fields has to be accessed using proper macros.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 3baea6a..fea7a3a 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -1822,7 +1822,7 @@ static __u64 blocks_per_migr_unit(struct intel_super *super,
migr_chunk = migr_strip_blocks_resync(dev);
disks = imsm_num_data_members(dev, 0);
blocks_per_unit = stripes_per_unit * migr_chunk * disks;
- stripe = __le32_to_cpu(map->blocks_per_strip) * disks;
+ stripe = __le16_to_cpu(map->blocks_per_strip) * disks;
segment = blocks_per_unit / stripe;
block_rel = blocks_per_unit - segment * stripe;
parity_depth = parity_segment_depth(dev);
@@ -8716,7 +8716,7 @@ static int imsm_manage_reshape(
ndata = imsm_num_data_members(dev, 0);
odata = imsm_num_data_members(dev, 1);
- chunk = map_src->blocks_per_strip * 512;
+ chunk = __le16_to_cpu(map_src->blocks_per_strip) * 512;
old_data_stripe_length = odata * chunk;
migr_rec = super->migr_rec;
@@ -8765,7 +8765,8 @@ static int imsm_manage_reshape(
if ((current_position + next_step) > max_position)
next_step = max_position - current_position;
- start = (map_src->pba_of_lba0 + dev->reserved_blocks +
+ start = (__le32_to_cpu(map_src->pba_of_lba0) +
+ __le32_to_cpu(dev->reserved_blocks) +
current_position) * 512;
/* allign reading start to old geometry */
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 08/21] imsm: FIX: Calculate backup location based on metadata information
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (6 preceding siblings ...)
2011-06-08 16:10 ` [PATCH 07/21] imsm: FIX: Use macros to data access Adam Kwolek
@ 2011-06-08 16:10 ` Adam Kwolek
2011-06-08 16:10 ` [PATCH 09/21] imsm: FIX: Do not verify unused parameters Adam Kwolek
` (13 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:10 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Use metadata information to calculate backup write offset.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index fea7a3a..0d46132 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7850,7 +7850,7 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
write_offset = ((unsigned long long)
__le32_to_cpu(migr_rec->dest_1st_member_lba) +
- info->data_offset) * 512;
+ __le32_to_cpu(map_dest->pba_of_lba0)) * 512;
unit_len = __le32_to_cpu(migr_rec->dest_depth_per_unit) * 512;
if (posix_memalign((void **)&buf, 512, unit_len) != 0)
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 09/21] imsm: FIX: Do not verify unused parameters
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (7 preceding siblings ...)
2011-06-08 16:10 ` [PATCH 08/21] imsm: FIX: Calculate backup location based on metadata information Adam Kwolek
@ 2011-06-08 16:10 ` Adam Kwolek
2011-06-08 16:10 ` [PATCH 10/21] imsm: FIX: Do not use pba_of_lba0 for copy position calculation Adam Kwolek
` (12 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:10 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Parameters that are not used by imsm_manage_reshape() should not cause
failure of this function.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 0d46132..437975f 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8689,7 +8689,7 @@ static int imsm_manage_reshape(
unsigned long long start_buf_shift; /* [bytes] */
int degraded = 0;
- if (!fds || !offsets || !destfd || !destoffsets || !sra)
+ if (!fds || !offsets || !sra)
goto abort;
/* Find volume during the reshape */
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 10/21] imsm: FIX: Do not use pba_of_lba0 for copy position calculation
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (8 preceding siblings ...)
2011-06-08 16:10 ` [PATCH 09/21] imsm: FIX: Do not verify unused parameters Adam Kwolek
@ 2011-06-08 16:10 ` Adam Kwolek
2011-06-08 16:11 ` [PATCH 11/21] imsm: FIX: Remove unused parameter from save_backup_imsm() interface Adam Kwolek
` (11 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:10 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
imsm_manage_reshape() should not shift start copy position.
This offset is passed to manage reshape function /and it is used/
as input parameter in offsets table already.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 437975f..b22c7df 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8765,9 +8765,7 @@ static int imsm_manage_reshape(
if ((current_position + next_step) > max_position)
next_step = max_position - current_position;
- start = (__le32_to_cpu(map_src->pba_of_lba0) +
- __le32_to_cpu(dev->reserved_blocks) +
- current_position) * 512;
+ start = current_position * 512;
/* allign reading start to old geometry */
start_buf_shift = start % old_data_stripe_length;
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 11/21] imsm: FIX: Remove unused parameter from save_backup_imsm() interface
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (9 preceding siblings ...)
2011-06-08 16:10 ` [PATCH 10/21] imsm: FIX: Do not use pba_of_lba0 for copy position calculation Adam Kwolek
@ 2011-06-08 16:11 ` Adam Kwolek
2011-06-08 16:11 ` [PATCH 12/21] imsm: FIX: Use metadata information for restore_stripes() and save_stripes() Adam Kwolek
` (10 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:11 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
new_data parameter is not used in save_backup_imsm().
It is removed from function interface.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index b22c7df..708b51d 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7677,9 +7677,9 @@ void init_migr_record_imsm(struct supertype *st, struct imsm_dev *dev,
* and to write it to the Copy Area.
* Parameters:
* st : supertype information
+ * dev : imsm device that backup is saved for
* info : general array info
* buf : input buffer
- * write_offset : address of data to backup
* length : length of data to backup (blocks_per_unit)
* Returns:
* 0 : success
@@ -7689,7 +7689,6 @@ int save_backup_imsm(struct supertype *st,
struct imsm_dev *dev,
struct mdinfo *info,
void *buf,
- int new_data,
int length)
{
int rv = -1;
@@ -8811,8 +8810,7 @@ static int imsm_manage_reshape(
* in backup general migration area
*/
if (save_backup_imsm(st, dev, sra,
- buf + start_buf_shift,
- ndata, copy_length)) {
+ buf + start_buf_shift, copy_length)) {
dprintf("imsm: Cannot save stripes to "
"target devices\n");
goto abort;
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 12/21] imsm: FIX: Use metadata information for restore_stripes() and save_stripes()
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (10 preceding siblings ...)
2011-06-08 16:11 ` [PATCH 11/21] imsm: FIX: Remove unused parameter from save_backup_imsm() interface Adam Kwolek
@ 2011-06-08 16:11 ` Adam Kwolek
2011-06-08 16:11 ` [PATCH 13/21] imsm: FIX: Detect failed devices during recover_backup_imsm() Adam Kwolek
` (9 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:11 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
For raid0 reshape imsm uses degraded raid4 for this operation.
Using real raid level (raid0) for stripe calculation causes no need
for parity calculation and can speed up reshape process.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 25 +++++++++++++++++--------
1 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 708b51d..26083c3 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7698,6 +7698,8 @@ int save_backup_imsm(struct supertype *st,
int i;
struct imsm_map *map_dest = get_imsm_map(dev, 0);
int new_disks = map_dest->num_members;
+ int dest_layout = 0;
+ int dest_chunk;
targets = malloc(new_disks * sizeof(int));
if (!targets)
@@ -7716,15 +7718,19 @@ int save_backup_imsm(struct supertype *st,
if (open_backup_targets(info, new_disks, targets))
goto abort;
+ if (map_dest->raid_level != 0)
+ dest_layout = ALGORITHM_LEFT_ASYMMETRIC;
+ dest_chunk = __le16_to_cpu(map_dest->blocks_per_strip) * 512;
+
if (restore_stripes(targets, /* list of dest devices */
target_offsets, /* migration record offsets */
new_disks,
- info->new_chunk,
- info->new_level,
- info->new_layout,
- -1, /* source backup file descriptor */
- 0, /* input buf offset
- * always 0 buf is already offset */
+ dest_chunk,
+ map_dest->raid_level,
+ dest_layout,
+ -1, /* source backup file descriptor */
+ 0, /* input buf offset
+ * always 0 buf is already offseted */
0,
length,
buf) != 0) {
@@ -8687,6 +8693,7 @@ static int imsm_manage_reshape(
unsigned long long start; /* [bytes] */
unsigned long long start_buf_shift; /* [bytes] */
int degraded = 0;
+ int source_layout = 0;
if (!fds || !offsets || !sra)
goto abort;
@@ -8741,6 +8748,8 @@ static int imsm_manage_reshape(
}
max_position = sra->component_size * ndata;
+ if (map_src->raid_level != 0)
+ source_layout = ALGORITHM_LEFT_ASYMMETRIC;
while (__le32_to_cpu(migr_rec->curr_migr_unit) <
__le32_to_cpu(migr_rec->num_migr_units)) {
@@ -8797,8 +8806,8 @@ static int imsm_manage_reshape(
start_buf_shift, next_step_filler);
if (save_stripes(fds, offsets, map_src->num_members,
- chunk, sra->array.level,
- sra->array.layout, 0, NULL, start_src,
+ chunk, map_src->raid_level,
+ source_layout, 0, NULL, start_src,
copy_length +
next_step_filler + start_buf_shift,
buf)) {
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 13/21] imsm: FIX: Detect failed devices during recover_backup_imsm()
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (11 preceding siblings ...)
2011-06-08 16:11 ` [PATCH 12/21] imsm: FIX: Use metadata information for restore_stripes() and save_stripes() Adam Kwolek
@ 2011-06-08 16:11 ` Adam Kwolek
2011-06-08 16:11 ` [PATCH 14/21] imsm: FIX: Move reshape_progress forward Adam Kwolek
` (8 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:11 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Detect in recover_backup_imsm() if not opened disks number is smaller
than allowed degradation for given raid level. This allows for reshape restart
on degraded array.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 26083c3..c19ffac 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -7826,6 +7826,8 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
unsigned long num_migr_units = __le32_to_cpu(migr_rec->num_migr_units);
int ascending = __le32_to_cpu(migr_rec->ascending_migr);
char buffer[20];
+ int skipped_disks = 0;
+ int max_degradation;
err = sysfs_get_str(info, NULL, "array_state", (char *)buffer, 20);
if (err < 1)
@@ -7849,6 +7851,7 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
map_dest = get_imsm_map(id->dev, 0);
new_disks = map_dest->num_members;
+ max_degradation = new_disks - imsm_num_data_members(id->dev, 0);
read_offset = (unsigned long long)
__le32_to_cpu(migr_rec->ckpt_area_pba) * 512;
@@ -7867,6 +7870,10 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
open_backup_targets(info, new_disks, targets);
for (i = 0; i < new_disks; i++) {
+ if (targets[i] < 0) {
+ skipped_disks++;
+ continue;
+ }
if (lseek64(targets[i], read_offset, SEEK_SET) < 0) {
fprintf(stderr,
Name ": Cannot seek to block: %s\n",
@@ -7893,6 +7900,13 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
}
}
+ if (skipped_disks > max_degradation) {
+ fprintf(stderr,
+ Name ": Cannot restore data from backup."
+ " Too many failed disks\n");
+ goto abort;
+ }
+
if (ascending && curr_migr_unit < (num_migr_units-1))
curr_migr_unit++;
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 14/21] imsm: FIX: Move reshape_progress forward
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (12 preceding siblings ...)
2011-06-08 16:11 ` [PATCH 13/21] imsm: FIX: Detect failed devices during recover_backup_imsm() Adam Kwolek
@ 2011-06-08 16:11 ` Adam Kwolek
2011-06-08 16:11 ` [PATCH 15/21] imsm: FIX: Remove unused variables and code Adam Kwolek
` (7 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:11 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
When array under reshape is assembled, reshape position used in sysfs_set_array()
should be set to position after recovered from backup area.
This avoids data corruption due to reshape the same array area again.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 23 +++++++++++++----------
1 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index c19ffac..55829cf 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -2194,6 +2194,13 @@ static void getinfo_super_imsm_volume(struct supertype *st, struct mdinfo *info,
unsigned long long array_blocks;
int used_disks;
+ if (__le32_to_cpu(migr_rec->ascending_migr) &&
+ (units <
+ (__le32_to_cpu(migr_rec->num_migr_units)-1)) &&
+ (super->migr_rec->rec_status ==
+ __cpu_to_le32(UNIT_SRC_IN_CP_AREA)))
+ units++;
+
info->reshape_progress = blocks_per_unit * units;
dprintf("IMSM: General Migration checkpoint : %llu "
@@ -7824,7 +7831,6 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
int retval = 1;
unsigned long curr_migr_unit = __le32_to_cpu(migr_rec->curr_migr_unit);
unsigned long num_migr_units = __le32_to_cpu(migr_rec->num_migr_units);
- int ascending = __le32_to_cpu(migr_rec->ascending_migr);
char buffer[20];
int skipped_disks = 0;
int max_degradation;
@@ -7907,16 +7913,13 @@ int recover_backup_imsm(struct supertype *st, struct mdinfo *info)
goto abort;
}
- if (ascending && curr_migr_unit < (num_migr_units-1))
- curr_migr_unit++;
-
- migr_rec->curr_migr_unit = __le32_to_cpu(curr_migr_unit);
- super->migr_rec->rec_status = __cpu_to_le32(UNIT_SRC_NORMAL);
- if (write_imsm_migr_rec(st) == 0) {
- __u64 blocks_per_unit = blocks_per_migr_unit(super, id->dev);
- info->reshape_progress = curr_migr_unit * blocks_per_unit;
+ if (save_checkpoint_imsm(st, info, UNIT_SRC_NORMAL)) {
+ /* ignore error == 2, this can mean end of reshape here
+ */
+ dprintf("imsm: Cannot write checkpoint to "
+ "migration record (UNIT_SRC_NORMAL) during restart\n");
+ } else
retval = 0;
- }
abort:
if (targets) {
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 15/21] imsm: FIX: Remove unused variables and code
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (13 preceding siblings ...)
2011-06-08 16:11 ` [PATCH 14/21] imsm: FIX: Move reshape_progress forward Adam Kwolek
@ 2011-06-08 16:11 ` Adam Kwolek
2011-06-08 16:11 ` [PATCH 16/21] FIX: Move buffer to next location Adam Kwolek
` (6 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:11 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Unused variables and code can be removed from imsm_manage_reshape()
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 14 +-------------
1 files changed, 1 insertions(+), 13 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 55829cf..2dd73c0 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8695,7 +8695,7 @@ static int imsm_manage_reshape(
struct intel_super *super = st->sb;
struct intel_dev *dv = NULL;
struct imsm_dev *dev = NULL;
- struct imsm_map *map_src, *map_dest;
+ struct imsm_map *map_src;
int migr_vol_qan = 0;
int ndata, odata; /* [bytes] */
int chunk; /* [bytes] */
@@ -8705,7 +8705,6 @@ static int imsm_manage_reshape(
unsigned long long max_position; /* array size [bytes] */
unsigned long long next_step; /* [blocks]/[bytes] */
unsigned long long old_data_stripe_length;
- unsigned long long new_data_stripe_length;
unsigned long long start_src; /* [bytes] */
unsigned long long start; /* [bytes] */
unsigned long long start_buf_shift; /* [bytes] */
@@ -8734,7 +8733,6 @@ static int imsm_manage_reshape(
map_src = get_imsm_map(dev, 1);
if (map_src == NULL)
goto abort;
- map_dest = get_imsm_map(dev, 0);
ndata = imsm_num_data_members(dev, 0);
odata = imsm_num_data_members(dev, 1);
@@ -8744,11 +8742,6 @@ static int imsm_manage_reshape(
migr_rec = super->migr_rec;
- /* [bytes] */
- sra->new_chunk = __le16_to_cpu(map_dest->blocks_per_strip) * 512;
- sra->new_level = map_dest->raid_level;
- new_data_stripe_length = sra->new_chunk * ndata;
-
/* initialize migration record for start condition */
if (sra->reshape_progress == 0)
init_migr_record_imsm(st, dev, sra);
@@ -8847,11 +8840,6 @@ static int imsm_manage_reshape(
"migration record (UNIT_SRC_IN_CP_AREA)\n");
goto abort;
}
- /* decrease backup_blocks */
- if (backup_blocks > (unsigned long)next_step)
- backup_blocks -= next_step;
- else
- backup_blocks = 0;
}
/* When data backed up, checkpoint stored,
* kick the kernel to reshape unit of data
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 16/21] FIX: Move buffer to next location
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (14 preceding siblings ...)
2011-06-08 16:11 ` [PATCH 15/21] imsm: FIX: Remove unused variables and code Adam Kwolek
@ 2011-06-08 16:11 ` Adam Kwolek
2011-06-08 16:11 ` [PATCH 17/21] imsm: FIX: Do not continue reshape when backup exists Adam Kwolek
` (5 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:11 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
When no output file is given save_stripes() should collect amount of stripes
in passed buffer. Currently all stripes are saved in the same area in passed
buffer. This causes that last stripe is returned on buffer begin only.
Increase buffer (buf) pointer when save_stripes() is about switch to next
stripe operation. This allows for proper buffer filling as input parameter
length directs.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
---
restripe.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/restripe.c b/restripe.c
index 1c42b60..79c695d 100644
--- a/restripe.c
+++ b/restripe.c
@@ -652,10 +652,14 @@ int save_stripes(int *source, unsigned long long *offsets,
fdisk[0], fdisk[1], bufs);
}
}
- if (dest)
+ if (dest) {
for (i = 0; i < nwrites; i++)
if (write(dest[i], buf, len) != len)
return -1;
+ } else {
+ /* build next stripe in buffer */
+ buf += len;
+ }
length -= len;
start += len;
}
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 17/21] imsm: FIX: Do not continue reshape when backup exists
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (15 preceding siblings ...)
2011-06-08 16:11 ` [PATCH 16/21] FIX: Move buffer to next location Adam Kwolek
@ 2011-06-08 16:11 ` Adam Kwolek
2011-06-08 16:11 ` [PATCH 18/21] imsm: FIX: wait_for_reshape_imsm() cleanup Adam Kwolek
` (4 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:11 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
When backup exists in copy area reshape cannot be continued.
In such situation, array is in unstable state.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 2dd73c0..25e706f 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8745,6 +8745,13 @@ static int imsm_manage_reshape(
/* initialize migration record for start condition */
if (sra->reshape_progress == 0)
init_migr_record_imsm(st, dev, sra);
+ else {
+ if (__le32_to_cpu(migr_rec->rec_status) != UNIT_SRC_NORMAL) {
+ dprintf("imsm: cannot restart migration when data "
+ "are present in copy area.\n");
+ goto abort;
+ }
+ }
/* size for data */
buf_size = __le32_to_cpu(migr_rec->blocks_per_unit) * 512;
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 18/21] imsm: FIX: wait_for_reshape_imsm() cleanup
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (16 preceding siblings ...)
2011-06-08 16:11 ` [PATCH 17/21] imsm: FIX: Do not continue reshape when backup exists Adam Kwolek
@ 2011-06-08 16:11 ` Adam Kwolek
2011-06-08 16:12 ` [PATCH 19/21] imsm: FIX: Remove timeout from wait_for_reshape_imsm() Adam Kwolek
` (3 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:11 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
This function needs to be corrected.
It should check sysfs operations status and it should not interpret
0 reshape position special meaning.
Unused input parameter is removed also.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 54 +++++++++++++++++++++++++++++++++---------------------
1 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 25e706f..8806339 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8560,39 +8560,50 @@ exit_imsm_reshape_super:
* reshape process reach new position
* Parameters:
* sra : general array info
- * to_complete : new sync_max position
* ndata : number of disks in new array's layout
* Returns:
* 0 : success,
* 1 : there is no reshape in progress,
* -1 : fail
******************************************************************************/
-int wait_for_reshape_imsm(struct mdinfo *sra, unsigned long long to_complete,
- int ndata)
+int wait_for_reshape_imsm(struct mdinfo *sra, int ndata)
{
int fd = sysfs_get_fd(sra, NULL, "reshape_position");
unsigned long long completed;
+ /* to_complete : new sync_max position */
+ unsigned long long to_complete = sra->reshape_progress;
+ unsigned long long position_to_set = to_complete / ndata;
struct timeval timeout;
- if (fd < 0)
+ if (fd < 0) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "cannot open reshape_position\n");
return 1;
+ }
- sysfs_fd_get_ll(fd, &completed);
+ if (sysfs_fd_get_ll(fd, &completed) < 0) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "cannot read reshape_position (no reshape in progres)\n");
+ close(fd);
+ return 0;
+ }
- if (to_complete == 0) {/* reshape till the end of array */
- sysfs_set_str(sra, NULL, "sync_max", "max");
- to_complete = MaxSector;
- } else {
- if (completed > to_complete) {
- close(fd);
- return -1;
- }
- if (sysfs_set_num(sra, NULL, "sync_max",
- to_complete / ndata) != 0) {
- close(fd);
- return -1;
- }
+ if (completed > to_complete) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "wrong next position to set %llu (%llu)\n",
+ to_complete, completed);
+ close(fd);
+ return -1;
+ }
+ dprintf("Position set: %llu\n", position_to_set);
+ if (sysfs_set_num(sra, NULL, "sync_max",
+ position_to_set) != 0) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "cannot set reshape position to %llu\n",
+ position_to_set);
+ close(fd);
+ return -1;
}
/* FIXME should not need a timeout at all */
@@ -8605,6 +8616,8 @@ int wait_for_reshape_imsm(struct mdinfo *sra, unsigned long long to_complete,
FD_SET(fd, &rfds);
select(fd+1, NULL, NULL, &rfds, &timeout);
if (sysfs_fd_get_ll(fd, &completed) < 0) {
+ dprintf("imsm: wait_for_reshape_imsm() "
+ "cannot read reshape_position (in loop)\n");
close(fd);
return 1;
}
@@ -8854,15 +8867,14 @@ static int imsm_manage_reshape(
next_step = next_step + sra->reshape_progress;
sysfs_set_num(sra, NULL, "suspend_lo", sra->reshape_progress);
sysfs_set_num(sra, NULL, "suspend_hi", next_step);
+ sra->reshape_progress = next_step;
/* wait until reshape finish */
- if (wait_for_reshape_imsm(sra, next_step, ndata) < 0) {
+ if (wait_for_reshape_imsm(sra, ndata) < 0) {
dprintf("wait_for_reshape_imsm returned error!\n");
goto abort;
}
- sra->reshape_progress = next_step;
-
if (save_checkpoint_imsm(st, sra, UNIT_SRC_NORMAL) == 1) {
/* ignore error == 2, this can mean end of reshape here
*/
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 19/21] imsm: FIX: Remove timeout from wait_for_reshape_imsm()
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (17 preceding siblings ...)
2011-06-08 16:11 ` [PATCH 18/21] imsm: FIX: wait_for_reshape_imsm() cleanup Adam Kwolek
@ 2011-06-08 16:12 ` Adam Kwolek
2011-06-08 16:12 ` [PATCH 20/21] imsm: Optimize expansion speed when no backup is required Adam Kwolek
` (2 subsequent siblings)
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:12 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Timeout should not be used for select function in wait_for_reshape_imsm().
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 15 +++++----------
1 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index 8806339..fae1218 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8574,8 +8574,6 @@ int wait_for_reshape_imsm(struct mdinfo *sra, int ndata)
unsigned long long to_complete = sra->reshape_progress;
unsigned long long position_to_set = to_complete / ndata;
- struct timeval timeout;
-
if (fd < 0) {
dprintf("imsm: wait_for_reshape_imsm() "
"cannot open reshape_position\n");
@@ -8606,25 +8604,22 @@ int wait_for_reshape_imsm(struct mdinfo *sra, int ndata)
return -1;
}
- /* FIXME should not need a timeout at all */
- timeout.tv_sec = 30;
- timeout.tv_usec = 0;
do {
char action[20];
fd_set rfds;
FD_ZERO(&rfds);
FD_SET(fd, &rfds);
- select(fd+1, NULL, NULL, &rfds, &timeout);
+ select(fd+1, &rfds, NULL, NULL, NULL);
+ if (sysfs_get_str(sra, NULL, "sync_action",
+ action, 20) > 0 &&
+ strncmp(action, "reshape", 7) != 0)
+ break;
if (sysfs_fd_get_ll(fd, &completed) < 0) {
dprintf("imsm: wait_for_reshape_imsm() "
"cannot read reshape_position (in loop)\n");
close(fd);
return 1;
}
- if (sysfs_get_str(sra, NULL, "sync_action",
- action, 20) > 0 &&
- strncmp(action, "reshape", 7) != 0)
- break;
} while (completed < to_complete);
close(fd);
return 0;
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 20/21] imsm: Optimize expansion speed when no backup is required
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (18 preceding siblings ...)
2011-06-08 16:12 ` [PATCH 19/21] imsm: FIX: Remove timeout from wait_for_reshape_imsm() Adam Kwolek
@ 2011-06-08 16:12 ` Adam Kwolek
2011-06-08 16:12 ` [PATCH 21/21] MAN: Man update for check-pointing Adam Kwolek
2011-06-09 3:03 ` [PATCH 00/21] IMSM Checkpointing Bug Fix Series NeilBrown
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:12 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
When no reshape backup is required (e.g. OLCE after critical section),
check-pointing can use bigger steps than backup space allows for.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
super-intel.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/super-intel.c b/super-intel.c
index fae1218..71a1189 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -8855,11 +8855,19 @@ static int imsm_manage_reshape(
"migration record (UNIT_SRC_IN_CP_AREA)\n");
goto abort;
}
+ } else {
+ /* set next step to use whole border area */
+ border /= next_step;
+ if (border > 1)
+ next_step *= border;
}
/* When data backed up, checkpoint stored,
* kick the kernel to reshape unit of data
*/
next_step = next_step + sra->reshape_progress;
+ /* limit next step to array max position */
+ if (next_step > max_position)
+ next_step = max_position;
sysfs_set_num(sra, NULL, "suspend_lo", sra->reshape_progress);
sysfs_set_num(sra, NULL, "suspend_hi", next_step);
sra->reshape_progress = next_step;
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 21/21] MAN: Man update for check-pointing
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (19 preceding siblings ...)
2011-06-08 16:12 ` [PATCH 20/21] imsm: Optimize expansion speed when no backup is required Adam Kwolek
@ 2011-06-08 16:12 ` Adam Kwolek
2011-06-09 3:03 ` [PATCH 00/21] IMSM Checkpointing Bug Fix Series NeilBrown
21 siblings, 0 replies; 25+ messages in thread
From: Adam Kwolek @ 2011-06-08 16:12 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
mdadm.8.in | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/mdadm.8.in b/mdadm.8.in
index e1d5651..f549dbf 100644
--- a/mdadm.8.in
+++ b/mdadm.8.in
@@ -2225,8 +2225,8 @@ succeed.
This is for the following reasons:
.IP 1.
-Intel's native IMSM check-pointing is not fully implemented yet.
-This causes IMSM incompatibility during the grow process: an array
+Intel's native IMSM check-pointing is not fully tested yet.
+This can causes IMSM incompatibility during the grow process: an array
which is growing cannot roam between Microsoft Windows(R) and Linux
systems.
@@ -2234,6 +2234,11 @@ systems.
Interrupting a grow operation is not recommended, because it
has not been fully tested for Intel's IMSM container format yet.
+.PP
+Note: Intel's native checkpointing doesn't use
+.B --backup-file
+option and it is transparent for assembly feature.
+
.SS SIZE CHANGES
Normally when an array is built the "size" is taken from the smallest
of the drives. If all the small drives in an arrays are, one at a
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 00/21] IMSM Checkpointing Bug Fix Series
2011-06-08 16:09 [PATCH 00/21] IMSM Checkpointing Bug Fix Series Adam Kwolek
` (20 preceding siblings ...)
2011-06-08 16:12 ` [PATCH 21/21] MAN: Man update for check-pointing Adam Kwolek
@ 2011-06-09 3:03 ` NeilBrown
21 siblings, 0 replies; 25+ messages in thread
From: NeilBrown @ 2011-06-09 3:03 UTC (permalink / raw)
To: Adam Kwolek
Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer
On Wed, 08 Jun 2011 18:09:33 +0200 Adam Kwolek <adam.kwolek@intel.com> wrote:
> The following series fixes problems found in IMSM's checkpointing.
> It contains rework based on Neil's comments to previous/initial checkpointing
> series and tt should be applied on neil_master branch (on the top
> of previous checkpointing patches).
>
> BR
> Adam
>
>
> ---
>
> Adam Kwolek (21):
> MAN: Man update for check-pointing
> imsm: Optimize expansion speed when no backup is required
> imsm: FIX: Remove timeout from wait_for_reshape_imsm()
> imsm: FIX: wait_for_reshape_imsm() cleanup
> imsm: FIX: Do not continue reshape when backup exists
> FIX: Move buffer to next location
> imsm: FIX: Remove unused variables and code
> imsm: FIX: Move reshape_progress forward
> imsm: FIX: Detect failed devices during recover_backup_imsm()
> imsm: FIX: Use metadata information for restore_stripes() and save_stripes()
> imsm: FIX: Remove unused parameter from save_backup_imsm() interface
> imsm: FIX: Do not use pba_of_lba0 for copy position calculation
> imsm: FIX: Do not verify unused parameters
> imsm: FIX: Calculate backup location based on metadata information
> imsm: FIX: Use macros to data access
> imsm: FIX: Check layout for level migration
> imsm: FIX: Max position could not be rounded to MB
> imsm: FIX: Detect migration end during migration record saving
> imsm: FIX: Verify if migration record is loaded correctly
> imsm: FIX: Opened handle is not closed
> imsm: FIX: Cannot create volume
>
>
> mdadm.8.in | 9 ++
> restripe.c | 6 +
> super-intel.c | 237 ++++++++++++++++++++++++++++++++++-----------------------
> 3 files changed, 153 insertions(+), 99 deletions(-)
>
Thanks.
Apart from the first one which I have already commented on, these look good.
I have applied them all, thanks.
Please confirm that it all still works for you and let me know if you have
any other changes pending that I should know about.
Thanks,
NeilBrown
^ permalink raw reply [flat|nested] 25+ messages in thread