* [PATCH 01/10] FIX: Cannot exit monitor after takeover
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
@ 2010-12-02 8:18 ` Adam Kwolek
2010-12-02 8:18 ` [PATCH 02/10] FIX: Problem with removing array " Adam Kwolek
` (9 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:18 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
When performing backward takeover to raid0 monitor cannot exit
for single raid0 array configuration.
Monitor is locked by communication (ping_manager()) after unfreeze()
Do not ping manager for raid0 array as they shouldn't be monitored.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
msg.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/msg.c b/msg.c
index 5ddf6e3..5511ecd 100644
--- a/msg.c
+++ b/msg.c
@@ -381,6 +381,7 @@ void unblock_monitor(char *container, const int unfreeze)
{
struct mdstat_ent *ent, *e;
struct mdinfo *sra = NULL;
+ int to_ping = 0;
ent = mdstat_read(0, 0);
if (!ent) {
@@ -394,11 +395,14 @@ void unblock_monitor(char *container, const int unfreeze)
if (!is_container_member(e, container))
continue;
sysfs_free(sra);
- sra = sysfs_read(-1, e->devnum, GET_VERSION);
+ sra = sysfs_read(-1, e->devnum, GET_VERSION|GET_LEVEL);
+ if (sra->array.level > 0)
+ to_ping++;
if (unblock_subarray(sra, unfreeze))
fprintf(stderr, Name ": Failed to unfreeze %s\n", e->dev);
}
- ping_monitor(container);
+ if (to_ping)
+ ping_monitor(container);
sysfs_free(sra);
free_mdstat(ent);
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 02/10] FIX: Problem with removing array after takeover
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
2010-12-02 8:18 ` [PATCH 01/10] FIX: Cannot exit monitor after takeover Adam Kwolek
@ 2010-12-02 8:18 ` Adam Kwolek
2010-12-03 3:46 ` Neil Brown
2010-12-02 8:19 ` [PATCH 03/10] FIX: Add error code for raid_disks set Adam Kwolek
` (8 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:18 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
When array parameters are changed old array 'A' is going to be removed
and new array 'B' is going to be serviced. If array B is raid0 array (takeovered),
array 'A' will never be deleted and mdmon is not going to exit.
Scenario:
1. managemon creates array 'B' and inserts it to begin of active arrays list
2. managemon sets field B->replaces = A
3. monitor: finds that array 'B' is raid 0 array and removes it from list
information about removing array 'A' from list is lost
and array 'A' stays in list forever
To resolve this situation first try to remove replaced arrays and then proceed
regular array servicing.
Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
monitor.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/monitor.c b/monitor.c
index 59b4181..0fbe198 100644
--- a/monitor.c
+++ b/monitor.c
@@ -480,6 +480,22 @@ static int wait_and_act(struct supertype *container, int nowait)
for (ap = aap ; *ap ;) {
a = *ap;
+
+ /* Remove array that current array points to remove
+ */
+ if (a->replaces && !discard_this) {
+ struct active_array **ap;
+ for (ap = &a->next; *ap && *ap != a->replaces;
+ ap = & (*ap)->next)
+ ;
+ if (*ap)
+ *ap = (*ap)->next;
+ discard_this = a->replaces;
+ a->replaces = NULL;
+ /* FIXME check if device->state_fd need to be cleared?*/
+ signal_manager();
+ }
+
/* once an array has been deactivated we want to
* ask the manager to discard it.
*/
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 02/10] FIX: Problem with removing array after takeover
2010-12-02 8:18 ` [PATCH 02/10] FIX: Problem with removing array " Adam Kwolek
@ 2010-12-03 3:46 ` Neil Brown
0 siblings, 0 replies; 20+ messages in thread
From: Neil Brown @ 2010-12-03 3:46 UTC (permalink / raw)
To: Adam Kwolek; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
On Thu, 02 Dec 2010 09:18:56 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:
> When array parameters are changed old array 'A' is going to be removed
> and new array 'B' is going to be serviced. If array B is raid0 array (takeovered),
> array 'A' will never be deleted and mdmon is not going to exit.
> Scenario:
> 1. managemon creates array 'B' and inserts it to begin of active arrays list
> 2. managemon sets field B->replaces = A
>
> 3. monitor: finds that array 'B' is raid 0 array and removes it from list
> information about removing array 'A' from list is lost
> and array 'A' stays in list forever
>
> To resolve this situation first try to remove replaced arrays and then proceed
> regular array servicing.
>
> Signed-off-by: Krzysztof Wojcik <krzysztof.wojcik@intel.com>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
This doesn't look right. You are duplicating a chunk of code, and having two
copies of it is dangerous and confusing.
Based on your description of the issue, maybe it would make sense to avoid
step 3. i.e. if monitor find array 'B' which is RAID0, it doesn't remove it
from the list while it still has a 'replaces' pointer. Obviously monitor
would need to be careful not to do anything else with the array. I think the
'remove it if it is RAID0' is in code that I having accepted yet, so I cannot
easily check if that does make sense.. What do you think?
I won't apply this patch at this stage though.
Thanks,
NeilBrown
> ---
>
> monitor.c | 16 ++++++++++++++++
> 1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 59b4181..0fbe198 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -480,6 +480,22 @@ static int wait_and_act(struct supertype *container, int nowait)
>
> for (ap = aap ; *ap ;) {
> a = *ap;
> +
> + /* Remove array that current array points to remove
> + */
> + if (a->replaces && !discard_this) {
> + struct active_array **ap;
> + for (ap = &a->next; *ap && *ap != a->replaces;
> + ap = & (*ap)->next)
> + ;
> + if (*ap)
> + *ap = (*ap)->next;
> + discard_this = a->replaces;
> + a->replaces = NULL;
> + /* FIXME check if device->state_fd need to be cleared?*/
> + signal_manager();
> + }
> +
> /* once an array has been deactivated we want to
> * ask the manager to discard it.
> */
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 03/10] FIX: Add error code for raid_disks set
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
2010-12-02 8:18 ` [PATCH 01/10] FIX: Cannot exit monitor after takeover Adam Kwolek
2010-12-02 8:18 ` [PATCH 02/10] FIX: Problem with removing array " Adam Kwolek
@ 2010-12-02 8:19 ` Adam Kwolek
2010-12-02 18:56 ` Dan Williams
2010-12-02 8:19 ` [PATCH 04/10] Add support to skip slot configuration Adam Kwolek
` (7 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:19 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
If error occurs mdadm exits with no information.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
Grow.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/Grow.c b/Grow.c
index 6e88d6d..eb0c7e0 100644
--- a/Grow.c
+++ b/Grow.c
@@ -1386,6 +1386,9 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
count = reshape_container_raid_disks(container, raid_disks);
if (count < 0) {
revert_container_raid_disks(st, fd, container);
+ fprintf(stderr, Name
+ ": unable to set requested raid disks number = %i\n",
+ raid_disks);
rv = 1;
goto release;
} else if (count == 0) {
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 03/10] FIX: Add error code for raid_disks set
2010-12-02 8:19 ` [PATCH 03/10] FIX: Add error code for raid_disks set Adam Kwolek
@ 2010-12-02 18:56 ` Dan Williams
0 siblings, 0 replies; 20+ messages in thread
From: Dan Williams @ 2010-12-02 18:56 UTC (permalink / raw)
To: Adam Kwolek; +Cc: neilb, linux-raid, ed.ciechanowski
On Thu, Dec 2, 2010 at 12:19 AM, Adam Kwolek <adam.kwolek@intel.com> wrote:
> If error occurs mdadm exits with no information.
>
Unless I am missing another patch all return paths currently have an
error message?
...
if (!ent) {
fprintf(stderr, Name ": unable to read /proc/mdstat\n");
return -1;
}
...
if (rv) {
fprintf(stderr, Name
": failed to initiate container reshape%s%s\n",
err ? ": " : "", err ? strerror(err) : "");
return rv;
}
return changed;
...and 'changed' is never negative.
--
Dan
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 04/10] Add support to skip slot configuration
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
` (2 preceding siblings ...)
2010-12-02 8:19 ` [PATCH 03/10] FIX: Add error code for raid_disks set Adam Kwolek
@ 2010-12-02 8:19 ` Adam Kwolek
2010-12-02 8:19 ` [PATCH 05/10] Add spares to raid0 array using takeover Adam Kwolek
` (6 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:19 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
When disk is added, set valid slot numbers (positive) only.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
sysfs.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sysfs.c b/sysfs.c
index 16e41fb..7a0403d 100644
--- a/sysfs.c
+++ b/sysfs.c
@@ -614,7 +614,8 @@ int sysfs_add_disk(struct mdinfo *sra, struct mdinfo *sd, int resume)
* yet, so just ignore status for now.
*/
sysfs_set_str(sra, sd, "state", "insync");
- rv |= sysfs_set_num(sra, sd, "slot", sd->disk.raid_disk);
+ if (sd->disk.raid_disk >= 0)
+ rv |= sysfs_set_num(sra, sd, "slot", sd->disk.raid_disk);
if (resume)
sysfs_set_num(sra, sd, "recovery_start", sd->recovery_start);
}
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 05/10] Add spares to raid0 array using takeover
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
` (3 preceding siblings ...)
2010-12-02 8:19 ` [PATCH 04/10] Add support to skip slot configuration Adam Kwolek
@ 2010-12-02 8:19 ` Adam Kwolek
2010-12-03 3:52 ` Neil Brown
2010-12-02 8:19 ` [PATCH 06/10] FIX: open backup file for reshape as function Adam Kwolek
` (5 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:19 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
Spares are used by Online Capacity Expansion to expand array.
To run expansion on raid0, spares have to be added to raid0 volume also.
Raid0 cannot have spares (no mdmon runs for raid0 array).
To do this, takeover to raid5 (and back) is used. mdmon runs temporary for raid5 and spare drives can be added to container.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
Manage.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 119 insertions(+), 1 deletions(-)
diff --git a/Manage.c b/Manage.c
index a203ec9..8c04ba9 100644
--- a/Manage.c
+++ b/Manage.c
@@ -31,6 +31,103 @@
#define START_MD _IO (MD_MAJOR, 2)
#define STOP_MD _IO (MD_MAJOR, 3)
+
+void takeover5to0(struct mdinfo *sra)
+{
+ char *c;
+ int err;
+
+ dprintf("Takeover Raid5->Raid0.\n");
+
+ if (sra == NULL)
+ return;
+
+ c = map_num(pers, 0);
+ if (c == NULL)
+ return;
+
+ err = sysfs_set_str(sra, NULL, "level", c);
+
+ if (err)
+ fprintf(stderr,
+ Name ": %s: could not set level "
+ "to %s for external super.\n",
+ sra->sys_name, c);
+ sysfs_free(sra);
+}
+
+struct mdinfo *takeover0to5(int fd)
+{
+ struct mdinfo *ret_val = NULL;
+ int devnum;
+ struct mdinfo *sra = NULL;
+ struct mdstat_ent *mdstat = NULL;
+ struct mdstat_ent *m;
+ int dev_fd = -1;
+
+ dprintf("Takeover Raid0->Raid5.\n");
+ devnum = fd2devnum(fd);
+ if (mdmon_running(devnum)) {
+ dprintf("mdmon is runnig for this container - takeover is not required\n");
+ return ret_val;
+ }
+
+ mdstat = mdstat_read(0, 0);
+ for (m=mdstat; m; m=m->next) {
+ if (m->metadata_version &&
+ strncmp(m->metadata_version, "external:", 9)==0 &&
+ is_subarray(m->metadata_version+9) &&
+ devname2devnum(m->metadata_version+10) == devnum) {
+ if (strncmp(m->level, "raid0", 5) == 0) {
+ char *p = NULL;
+ char dev_name[PATH_MAX];
+ int err;
+
+ sprintf(dev_name, "/dev/md%i", m->devnum);
+ dev_fd = open_mddev(dev_name , 1);
+ if (dev_fd < 0)
+ continue;
+
+ sra = sysfs_read(dev_fd, 0, GET_VERSION|GET_LEVEL);
+ if (!sra)
+ break;
+
+ err = sysfs_set_str(sra, NULL, "level", "raid5");
+ if (err) {
+ fprintf(stderr, Name": %s: could not set level to "
+ "raid5 for external super.\n", sra->sys_name);
+ break;
+ }
+
+ /* return to this raid level and do not release
+ */
+ ret_val = sra;
+ sra = NULL;
+
+ /* if after takeover mdmon is not running,
+ * start it
+ */
+ if (!mdmon_running(devnum))
+ start_mdmon(devnum);
+ p = devnum2devname(devnum);
+ if (p) {
+ ping_monitor(p);
+ free(p);
+ }
+ sleep(1);
+ break;
+ }
+ }
+ }
+
+ sysfs_free(sra);
+ free_mdstat(mdstat);
+ if (dev_fd >= 0)
+ close(dev_fd);
+
+ return ret_val;
+}
+
int Manage_ro(char *devname, int fd, int readonly)
{
/* switch to readonly or rw
@@ -832,6 +929,7 @@ int Manage_subdevs(char *devname, int fd,
struct mdinfo *sra;
int container_fd;
int devnum = fd2devnum(fd);
+ char *devname = NULL;
container_fd = open_dev_excl(devnum);
if (container_fd < 0) {
@@ -840,10 +938,17 @@ int Manage_subdevs(char *devname, int fd,
dv->devname);
return 1;
}
+ /* Raid 0 add spare via takeover
+ */
+ struct mdinfo *return_raid0_sra = NULL;
+ /* try to perform takeover if needed
+ */
+ return_raid0_sra = takeover0to5(container_fd);
if (!mdmon_running(devnum)) {
fprintf(stderr, Name ": add failed for %s: mdmon not running\n",
dv->devname);
+ takeover5to0(return_raid0_sra);
close(container_fd);
return 1;
}
@@ -852,7 +957,13 @@ int Manage_subdevs(char *devname, int fd,
if (!sra) {
fprintf(stderr, Name ": add failed for %s: sysfs_read failed\n",
dv->devname);
+ takeover5to0(return_raid0_sra);
close(container_fd);
+ devname = devnum2devname(devnum);
+ if (devname) {
+ ping_monitor(devname);
+ free(devname);
+ }
return 1;
}
sra->array.level = LEVEL_CONTAINER;
@@ -864,12 +975,19 @@ int Manage_subdevs(char *devname, int fd,
if (sysfs_add_disk(sra, &new_mdi, 0) != 0) {
fprintf(stderr, Name ": add new device to external metadata"
" failed for %s\n", dv->devname);
+ takeover5to0(return_raid0_sra);
close(container_fd);
+ sysfs_free(sra);
return 1;
}
- ping_monitor(devnum2devname(devnum));
+ takeover5to0(return_raid0_sra);
sysfs_free(sra);
close(container_fd);
+ devname = devnum2devname(devnum);
+ if (devname) {
+ ping_monitor(devname);
+ free(devname);
+ }
} else if (ioctl(fd, ADD_NEW_DISK, &disc)) {
fprintf(stderr, Name ": add new device failed for %s as %d: %s\n",
dv->devname, j, strerror(errno));
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 05/10] Add spares to raid0 array using takeover
2010-12-02 8:19 ` [PATCH 05/10] Add spares to raid0 array using takeover Adam Kwolek
@ 2010-12-03 3:52 ` Neil Brown
0 siblings, 0 replies; 20+ messages in thread
From: Neil Brown @ 2010-12-03 3:52 UTC (permalink / raw)
To: Adam Kwolek; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
On Thu, 02 Dec 2010 09:19:20 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:
> Spares are used by Online Capacity Expansion to expand array.
> To run expansion on raid0, spares have to be added to raid0 volume also.
> Raid0 cannot have spares (no mdmon runs for raid0 array).
> To do this, takeover to raid5 (and back) is used. mdmon runs temporary for raid5 and spare drives can be added to container.
>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
This is just wrong.
If mdmon is running (maybe because some other array in the container needs
it) then you add a spare to the container and send mdmon a metadata update to
apply.
If mdmon is not running, then you add a spare to the container and apply a
metadata update directly in mdadm.
There is no need to switch to raid5 and back just to add a spare.
NeilBrown
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 06/10] FIX: open backup file for reshape as function
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
` (4 preceding siblings ...)
2010-12-02 8:19 ` [PATCH 05/10] Add spares to raid0 array using takeover Adam Kwolek
@ 2010-12-02 8:19 ` Adam Kwolek
2010-12-03 4:01 ` Neil Brown
2010-12-02 8:19 ` [PATCH 07/10] FIX: Do not use layout for raid4 and raid0 while geo map computing Adam Kwolek
` (4 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:19 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
Move opening backup file to the function for future reuse during container reshape.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
Grow.c | 109 ++++++++++++++++++++++++++++++++++++++-------------------------
mdadm.h | 8 ++++-
2 files changed, 73 insertions(+), 44 deletions(-)
diff --git a/Grow.c b/Grow.c
index eb0c7e0..96c9526 100644
--- a/Grow.c
+++ b/Grow.c
@@ -913,6 +913,66 @@ int reshape_prepare_fdlist(char *devname,
release:
return d;
}
+int reshape_open_backup_file(char *backup_file,
+ int fd,
+ char *devname,
+ int d,
+ long blocks,
+ int *fdlist,
+ unsigned long long *offsets)
+{
+ /* need to check backup file is large enough */
+ char buf[512];
+ struct stat stb;
+ unsigned int dev;
+ int ret_val = -1;
+ int i;
+
+ fdlist[d] = open(backup_file, O_RDWR|O_CREAT|O_EXCL,
+ S_IRUSR | S_IWUSR);
+ offsets[d] = 8 * 512;
+ if (fdlist[d] < 0) {
+ fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
+ devname, backup_file, strerror(errno));
+ ret_val = 1;
+ return ret_val;
+ }
+ /* Guard against backup file being on array device.
+ * If array is partitioned or if LVM etc is in the
+ * way this will not notice, but it is better than
+ * nothing.
+ */
+ fstat(fdlist[d], &stb);
+ dev = stb.st_dev;
+ fstat(fd, &stb);
+ if (stb.st_rdev == dev) {
+ fprintf(stderr, Name ": backup file must NOT be"
+ " on the array being reshaped.\n");
+ ret_val = 1;
+ close(fdlist[d]);
+ return ret_val;
+ }
+
+ memset(buf, 0, 512);
+ for (i=0; i < blocks + 1 ; i++) {
+ if (write(fdlist[d], buf, 512) != 512) {
+ fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
+ devname, backup_file, strerror(errno));
+ ret_val = 1;
+ return ret_val;
+ }
+ }
+ if (fsync(fdlist[d]) != 0) {
+ fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
+ devname, backup_file, strerror(errno));
+ ret_val = 1;
+ return ret_val;
+ }
+
+ ret_val = d + 1;
+
+ return ret_val;
+}
unsigned long compute_backup_blocks(int nchunk, int ochunk,
unsigned int ndata, unsigned int odata)
@@ -974,7 +1034,7 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
char alt_layout[40];
int *fdlist;
unsigned long long *offsets;
- int d, i;
+ int d;
int nrdisks;
int err;
int frozen;
@@ -1652,51 +1712,14 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
break;
}
} else {
- /* need to check backup file is large enough */
- char buf[512];
- struct stat stb;
- unsigned int dev;
- fdlist[d] = open(backup_file, O_RDWR|O_CREAT|O_EXCL,
- S_IRUSR | S_IWUSR);
- offsets[d] = 8 * 512;
- if (fdlist[d] < 0) {
- fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
- devname, backup_file, strerror(errno));
- rv = 1;
- break;
- }
- /* Guard against backup file being on array device.
- * If array is partitioned or if LVM etc is in the
- * way this will not notice, but it is better than
- * nothing.
- */
- fstat(fdlist[d], &stb);
- dev = stb.st_dev;
- fstat(fd, &stb);
- if (stb.st_rdev == dev) {
- fprintf(stderr, Name ": backup file must NOT be"
- " on the array being reshaped.\n");
- rv = 1;
- close(fdlist[d]);
- break;
- }
-
- memset(buf, 0, 512);
- for (i=0; i < (signed)blocks + 1 ; i++) {
- if (write(fdlist[d], buf, 512) != 512) {
- fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
- devname, backup_file, strerror(errno));
- rv = 1;
- break;
- }
- }
- if (fsync(fdlist[d]) != 0) {
- fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
- devname, backup_file, strerror(errno));
+ int handlers = reshape_open_backup_file(backup_file, fd, devname,
+ d, (signed)blocks,
+ fdlist, offsets);
+ if (handlers < 0) {
rv = 1;
break;
}
- d++;
+ d = handlers;
}
/* check that the operation is supported by the metadata */
diff --git a/mdadm.h b/mdadm.h
index a0126eb..839fb6c 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -486,7 +486,13 @@ extern int reshape_prepare_fdlist(char *devname,
extern void reshape_free_fdlist(int *fdlist,
unsigned long long *offsets,
int size);
-
+extern int reshape_open_backup_file(char *backup,
+ int fd,
+ char *devname,
+ int d,
+ long blocks,
+ int *fdlist,
+ unsigned long long *offsets);
extern unsigned long compute_backup_blocks(int nchunk, int ochunk,
unsigned int ndata, unsigned int odata);
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 06/10] FIX: open backup file for reshape as function
2010-12-02 8:19 ` [PATCH 06/10] FIX: open backup file for reshape as function Adam Kwolek
@ 2010-12-03 4:01 ` Neil Brown
0 siblings, 0 replies; 20+ messages in thread
From: Neil Brown @ 2010-12-03 4:01 UTC (permalink / raw)
To: Adam Kwolek; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
On Thu, 02 Dec 2010 09:19:27 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:
> Move opening backup file to the function for future reuse during container reshape.
>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
Your return-value handling from the new function was somewhat confused. It
would never return -1 for example.
I have tidied it up and applied the following. Note that I avoided passing
'd' to the function.
NeilBrown
commit e6e9d47b76227f4f30e27dcd00e6b0d815370b7c
Author: Adam Kwolek <adam.kwolek@intel.com>
Date: Fri Dec 3 15:00:16 2010 +1100
Grow: open backup file for reshape as function
Move opening backup file to the function for future reuse during
container reshape.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
Signed-off-by: NeilBrown <neilb@suse.de>
diff --git a/Grow.c b/Grow.c
index 6e88d6d..c408a92 100644
--- a/Grow.c
+++ b/Grow.c
@@ -914,6 +914,61 @@ release:
return d;
}
+int reshape_open_backup_file(char *backup_file,
+ int fd,
+ char *devname,
+ long blocks,
+ int *fdlist,
+ unsigned long long *offsets)
+{
+ /* Return 1 on success, 0 on any form of failure */
+ /* need to check backup file is large enough */
+ char buf[512];
+ struct stat stb;
+ unsigned int dev;
+ int i;
+
+ *fdlist = open(backup_file, O_RDWR|O_CREAT|O_EXCL,
+ S_IRUSR | S_IWUSR);
+ *offsets = 8 * 512;
+ if (*fdlist < 0) {
+ fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
+ devname, backup_file, strerror(errno));
+ return 0;
+ }
+ /* Guard against backup file being on array device.
+ * If array is partitioned or if LVM etc is in the
+ * way this will not notice, but it is better than
+ * nothing.
+ */
+ fstat(*fdlist, &stb);
+ dev = stb.st_dev;
+ fstat(fd, &stb);
+ if (stb.st_rdev == dev) {
+ fprintf(stderr, Name ": backup file must NOT be"
+ " on the array being reshaped.\n");
+ close(*fdlist);
+ return 0;
+ }
+
+ memset(buf, 0, 512);
+ for (i=0; i < blocks + 1 ; i++) {
+ if (write(*fdlist, buf, 512) != 512) {
+ fprintf(stderr, Name ": %s: cannot create"
+ " backup file %s: %s\n",
+ devname, backup_file, strerror(errno));
+ return 0;
+ }
+ }
+ if (fsync(*fdlist) != 0) {
+ fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
+ devname, backup_file, strerror(errno));
+ return 0;
+ }
+
+ return 1;
+}
+
unsigned long compute_backup_blocks(int nchunk, int ochunk,
unsigned int ndata, unsigned int odata)
{
@@ -974,7 +1029,7 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
char alt_layout[40];
int *fdlist;
unsigned long long *offsets;
- int d, i;
+ int d;
int nrdisks;
int err;
int frozen;
@@ -1649,47 +1704,9 @@ int Grow_reshape(char *devname, int fd, int quiet, char *backup_file,
break;
}
} else {
- /* need to check backup file is large enough */
- char buf[512];
- struct stat stb;
- unsigned int dev;
- fdlist[d] = open(backup_file, O_RDWR|O_CREAT|O_EXCL,
- S_IRUSR | S_IWUSR);
- offsets[d] = 8 * 512;
- if (fdlist[d] < 0) {
- fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
- devname, backup_file, strerror(errno));
- rv = 1;
- break;
- }
- /* Guard against backup file being on array device.
- * If array is partitioned or if LVM etc is in the
- * way this will not notice, but it is better than
- * nothing.
- */
- fstat(fdlist[d], &stb);
- dev = stb.st_dev;
- fstat(fd, &stb);
- if (stb.st_rdev == dev) {
- fprintf(stderr, Name ": backup file must NOT be"
- " on the array being reshaped.\n");
- rv = 1;
- close(fdlist[d]);
- break;
- }
-
- memset(buf, 0, 512);
- for (i=0; i < (signed)blocks + 1 ; i++) {
- if (write(fdlist[d], buf, 512) != 512) {
- fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
- devname, backup_file, strerror(errno));
- rv = 1;
- break;
- }
- }
- if (fsync(fdlist[d]) != 0) {
- fprintf(stderr, Name ": %s: cannot create backup file %s: %s\n",
- devname, backup_file, strerror(errno));
+ if (!reshape_open_backup_file(backup_file, fd, devname,
+ (signed)blocks,
+ fdlist+d, offsets+d)) {
rv = 1;
break;
}
diff --git a/mdadm.h b/mdadm.h
index a0126eb..175d228 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -486,7 +486,12 @@ extern int reshape_prepare_fdlist(char *devname,
extern void reshape_free_fdlist(int *fdlist,
unsigned long long *offsets,
int size);
-
+extern int reshape_open_backup_file(char *backup,
+ int fd,
+ char *devname,
+ long blocks,
+ int *fdlist,
+ unsigned long long *offsets);
extern unsigned long compute_backup_blocks(int nchunk, int ochunk,
unsigned int ndata, unsigned int odata);
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 07/10] FIX: Do not use layout for raid4 and raid0 while geo map computing
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
` (5 preceding siblings ...)
2010-12-02 8:19 ` [PATCH 06/10] FIX: open backup file for reshape as function Adam Kwolek
@ 2010-12-02 8:19 ` Adam Kwolek
2010-12-02 8:19 ` [PATCH 08/10] FIX: sync_completed_fd handler has to be closed Adam Kwolek
` (3 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:19 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
After takeover layout has no matter for computing geo map for raid0 and raid4.
Set layout to 0 for such cases.
It can happen after takeover operation when not whole array information is reread.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
restripe.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/restripe.c b/restripe.c
index 3074693..dcce3cc 100644
--- a/restripe.c
+++ b/restripe.c
@@ -43,6 +43,11 @@ static int geo_map(int block, unsigned long long stripe, int raid_disks,
*/
int pd;
+ /* layout has no matter for raid0 and raid4 */
+ if ((level == 0) ||
+ (level == 4))
+ layout = 0;
+
switch(level*100 + layout) {
case 000:
case 400:
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 08/10] FIX: sync_completed_fd handler has to be closed
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
` (6 preceding siblings ...)
2010-12-02 8:19 ` [PATCH 07/10] FIX: Do not use layout for raid4 and raid0 while geo map computing Adam Kwolek
@ 2010-12-02 8:19 ` Adam Kwolek
2010-12-02 8:19 ` [PATCH 09/10] FIX: Honor !reshape state on wait_reshape() entry Adam Kwolek
` (2 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:19 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
sync_completed_fd handler has to be closed when array is closing.
This is in pair to open handler code.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
managemon.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/managemon.c b/managemon.c
index 12a3792..df491ba 100644
--- a/managemon.c
+++ b/managemon.c
@@ -120,6 +120,7 @@ static void close_aa(struct active_array *aa)
close(aa->action_fd);
close(aa->info.state_fd);
close(aa->resync_start_fd);
+ close(aa->sync_completed_fd);
}
static void free_aa(struct active_array *aa)
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 09/10] FIX: Honor !reshape state on wait_reshape() entry
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
` (7 preceding siblings ...)
2010-12-02 8:19 ` [PATCH 08/10] FIX: sync_completed_fd handler has to be closed Adam Kwolek
@ 2010-12-02 8:19 ` Adam Kwolek
2010-12-03 4:11 ` Neil Brown
2010-12-02 8:19 ` [PATCH 10/10] FIX: wait_backup() sometimes hungs Adam Kwolek
2010-12-03 4:19 ` [PATCH 00/10] Pre-migration patch series Neil Brown
10 siblings, 1 reply; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:19 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
When wait_reshape() function starts it can occurs that reshape is finished already,
before wait_reshape() start. This can lead to wait for change state inside this function for a long time.
To avoid this before wait we should test if finish conditions are not reached already.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
Grow.c | 19 ++++++++++++-------
1 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/Grow.c b/Grow.c
index 96c9526..24c5c39 100644
--- a/Grow.c
+++ b/Grow.c
@@ -548,17 +548,22 @@ static void wait_reshape(struct mdinfo *sra)
int fd = sysfs_get_fd(sra, NULL, "sync_action");
char action[20];
- do {
+ if (fd < 0)
+ return;
+
+ if (sysfs_fd_get_str(fd, action, 20) < 0) {
+ close(fd);
+ return;
+ }
+ while (strncmp(action, "reshape", 7) == 0) {
fd_set rfds;
FD_ZERO(&rfds);
FD_SET(fd, &rfds);
select(fd+1, NULL, NULL, &rfds, NULL);
-
- if (sysfs_fd_get_str(fd, action, 20) < 0) {
- close(fd);
- return;
- }
- } while (strncmp(action, "reshape", 7) == 0);
+ if (sysfs_fd_get_str(fd, action, 20) < 0)
+ break;
+ }
+ close(fd);
}
static int reshape_super(struct supertype *st, long long size, int level,
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 09/10] FIX: Honor !reshape state on wait_reshape() entry
2010-12-02 8:19 ` [PATCH 09/10] FIX: Honor !reshape state on wait_reshape() entry Adam Kwolek
@ 2010-12-03 4:11 ` Neil Brown
0 siblings, 0 replies; 20+ messages in thread
From: Neil Brown @ 2010-12-03 4:11 UTC (permalink / raw)
To: Adam Kwolek; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
On Thu, 02 Dec 2010 09:19:51 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:
> When wait_reshape() function starts it can occurs that reshape is finished already,
> before wait_reshape() start. This can lead to wait for change state inside this function for a long time.
> To avoid this before wait we should test if finish conditions are not reached already.
>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
>
> Grow.c | 19 ++++++++++++-------
> 1 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 96c9526..24c5c39 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -548,17 +548,22 @@ static void wait_reshape(struct mdinfo *sra)
> int fd = sysfs_get_fd(sra, NULL, "sync_action");
> char action[20];
>
> - do {
> + if (fd < 0)
> + return;
> +
> + if (sysfs_fd_get_str(fd, action, 20) < 0) {
> + close(fd);
> + return;
> + }
> + while (strncmp(action, "reshape", 7) == 0) {
> fd_set rfds;
> FD_ZERO(&rfds);
> FD_SET(fd, &rfds);
> select(fd+1, NULL, NULL, &rfds, NULL);
> -
> - if (sysfs_fd_get_str(fd, action, 20) < 0) {
> - close(fd);
> - return;
> - }
> - } while (strncmp(action, "reshape", 7) == 0);
> + if (sysfs_fd_get_str(fd, action, 20) < 0)
> + break;
> + }
> + close(fd);
> }
>
> static int reshape_super(struct supertype *st, long long size, int level,
Thanks for this fix. There is room to tidy the code up even more. This is
what I have applied.
Thanks,
NeilBrown
commit 92a19f1a78e040202e3d067960e3b1ecc8162881
Author: Adam Kwolek <adam.kwolek@intel.com>
Date: Fri Dec 3 15:10:20 2010 +1100
FIX: Honor !reshape state on wait_reshape() entry
When wait_reshape() function starts it can occurs that reshape is
finished already, before wait_reshape() start. This can lead to wait
for change state inside this function for a long time. To avoid this
before wait we should test if finish conditions are not reached
already.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
Signed-off-by: NeilBrown <neilb@suse.de>
diff --git a/Grow.c b/Grow.c
index c408a92..3322cf7 100644
--- a/Grow.c
+++ b/Grow.c
@@ -548,17 +548,17 @@ static void wait_reshape(struct mdinfo *sra)
int fd = sysfs_get_fd(sra, NULL, "sync_action");
char action[20];
- do {
+ if (fd < 0)
+ return;
+
+ while (sysfs_fd_get_str(fd, action, 20) > 0 &&
+ strncmp(action, "reshape", 7) == 0) {
fd_set rfds;
FD_ZERO(&rfds);
FD_SET(fd, &rfds);
select(fd+1, NULL, NULL, &rfds, NULL);
-
- if (sysfs_fd_get_str(fd, action, 20) < 0) {
- close(fd);
- return;
- }
- } while (strncmp(action, "reshape", 7) == 0);
+ }
+ close(fd);
}
static int reshape_super(struct supertype *st, long long size, int level,
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 10/10] FIX: wait_backup() sometimes hungs
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
` (8 preceding siblings ...)
2010-12-02 8:19 ` [PATCH 09/10] FIX: Honor !reshape state on wait_reshape() entry Adam Kwolek
@ 2010-12-02 8:19 ` Adam Kwolek
2010-12-03 4:16 ` Neil Brown
2010-12-03 4:19 ` [PATCH 00/10] Pre-migration patch series Neil Brown
10 siblings, 1 reply; 20+ messages in thread
From: Adam Kwolek @ 2010-12-02 8:19 UTC (permalink / raw)
To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
Sometimes wait_backup() omits transition from reshape to iddle state and mdadm seams to be hung.
Add 1 sec. timeout wor waiting on select. This allows for wait_backup exit when reshape is ended.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
---
Grow.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/Grow.c b/Grow.c
index 24c5c39..e16b1ad 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2074,10 +2074,14 @@ static int wait_backup(struct mdinfo *sra,
sysfs_set_str(sra, NULL, "sync_action", "reshape");
do {
char action[20];
+ struct timeval t;
+
+ t.tv_sec = 1;
+ t.tv_usec = 0;
fd_set rfds;
FD_ZERO(&rfds);
FD_SET(fd, &rfds);
- select(fd+1, NULL, NULL, &rfds, NULL);
+ select(fd+1, NULL, NULL, &rfds, &t);
if (sysfs_fd_get_ll(fd, &completed) < 0) {
close(fd);
return -1;
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 10/10] FIX: wait_backup() sometimes hungs
2010-12-02 8:19 ` [PATCH 10/10] FIX: wait_backup() sometimes hungs Adam Kwolek
@ 2010-12-03 4:16 ` Neil Brown
2010-12-03 7:45 ` Kwolek, Adam
0 siblings, 1 reply; 20+ messages in thread
From: Neil Brown @ 2010-12-03 4:16 UTC (permalink / raw)
To: Adam Kwolek; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
On Thu, 02 Dec 2010 09:19:58 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:
> Sometimes wait_backup() omits transition from reshape to iddle state and mdadm seams to be hung.
> Add 1 sec. timeout wor waiting on select. This allows for wait_backup exit when reshape is ended.
>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> ---
>
> Grow.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/Grow.c b/Grow.c
> index 24c5c39..e16b1ad 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2074,10 +2074,14 @@ static int wait_backup(struct mdinfo *sra,
> sysfs_set_str(sra, NULL, "sync_action", "reshape");
> do {
> char action[20];
> + struct timeval t;
> +
> + t.tv_sec = 1;
> + t.tv_usec = 0;
> fd_set rfds;
> FD_ZERO(&rfds);
> FD_SET(fd, &rfds);
> - select(fd+1, NULL, NULL, &rfds, NULL);
> + select(fd+1, NULL, NULL, &rfds, &t);
> if (sysfs_fd_get_ll(fd, &completed) < 0) {
> close(fd);
> return -1;
Thanks. However I don't think the 1 second timeout is necessary. This is
really the same problem as the previous one. We just need to read
'completed' before the first 'select'. Like this.
Thanks,
NeilBrown
commit 97bef35459306dfd291f40bc5221ad20ab9c21ba
Author: Adam Kwolek <adam.kwolek@intel.com>
Date: Fri Dec 3 15:15:51 2010 +1100
FIX: wait_backup() sometimes hungs
Sometimes wait_backup() omits transition from reshape to idle state
and mdadm seams to be hung. So check the 'complete' count
*before* waiting rather than only after.
Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
Signed-off-by: NeilBrown <neilb@suse.de>
diff --git a/Grow.c b/Grow.c
index 3322cf7..99807b4 100644
--- a/Grow.c
+++ b/Grow.c
@@ -2058,12 +2058,17 @@ static int wait_backup(struct mdinfo *sra,
sysfs_set_num(sra, NULL, "sync_max", offset + blocks + blocks2);
if (offset == 0)
sysfs_set_str(sra, NULL, "sync_action", "reshape");
- do {
+
+ if (sysfs_fd_get_ll(fd, &completed) < 0) {
+ close(fd);
+ return -1;
+ }
+ while (completed < offset + blocks) {
char action[20];
fd_set rfds;
FD_ZERO(&rfds);
FD_SET(fd, &rfds);
- select(fd+1, NULL, NULL, &rfds, NULL);
+ select(fd+1, NULL, NULL, &rfds, &t);
if (sysfs_fd_get_ll(fd, &completed) < 0) {
close(fd);
return -1;
@@ -2072,7 +2077,7 @@ static int wait_backup(struct mdinfo *sra,
action, 20) > 0 &&
strncmp(action, "reshape", 7) != 0)
break;
- } while (completed < offset + blocks);
+ }
close(fd);
if (part) {
^ permalink raw reply related [flat|nested] 20+ messages in thread* RE: [PATCH 10/10] FIX: wait_backup() sometimes hungs
2010-12-03 4:16 ` Neil Brown
@ 2010-12-03 7:45 ` Kwolek, Adam
2010-12-03 10:35 ` Neil Brown
0 siblings, 1 reply; 20+ messages in thread
From: Kwolek, Adam @ 2010-12-03 7:45 UTC (permalink / raw)
To: Neil Brown; +Cc: linux-raid@vger.kernel.org, Williams, Dan J, Ciechanowski, Ed
I think you applied not a whole patch (no 't' definition and initialization), you missed:
+ struct timeval t;
+
+ t.tv_sec = 1;
+ t.tv_usec = 0;
so mdadm is not compiling
BR
Adam
> -----Original Message-----
> From: Neil Brown [mailto:neilb@suse.de]
> Sent: Friday, December 03, 2010 5:17 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed
> Subject: Re: [PATCH 10/10] FIX: wait_backup() sometimes hungs
>
> On Thu, 02 Dec 2010 09:19:58 +0100 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
>
> > Sometimes wait_backup() omits transition from reshape to iddle state
> and mdadm seams to be hung.
> > Add 1 sec. timeout wor waiting on select. This allows for wait_backup
> exit when reshape is ended.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> > Grow.c | 6 +++++-
> > 1 files changed, 5 insertions(+), 1 deletions(-)
> >
> > diff --git a/Grow.c b/Grow.c
> > index 24c5c39..e16b1ad 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -2074,10 +2074,14 @@ static int wait_backup(struct mdinfo *sra,
> > sysfs_set_str(sra, NULL, "sync_action", "reshape");
> > do {
> > char action[20];
> > + struct timeval t;
> > +
> > + t.tv_sec = 1;
> > + t.tv_usec = 0;
> > fd_set rfds;
> > FD_ZERO(&rfds);
> > FD_SET(fd, &rfds);
> > - select(fd+1, NULL, NULL, &rfds, NULL);
> > + select(fd+1, NULL, NULL, &rfds, &t);
> > if (sysfs_fd_get_ll(fd, &completed) < 0) {
> > close(fd);
> > return -1;
>
>
> Thanks. However I don't think the 1 second timeout is necessary. This
> is
> really the same problem as the previous one. We just need to read
> 'completed' before the first 'select'. Like this.
>
> Thanks,
> NeilBrown
>
> commit 97bef35459306dfd291f40bc5221ad20ab9c21ba
> Author: Adam Kwolek <adam.kwolek@intel.com>
> Date: Fri Dec 3 15:15:51 2010 +1100
>
> FIX: wait_backup() sometimes hungs
>
> Sometimes wait_backup() omits transition from reshape to idle state
> and mdadm seams to be hung. So check the 'complete' count
> *before* waiting rather than only after.
>
> Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> Signed-off-by: NeilBrown <neilb@suse.de>
>
> diff --git a/Grow.c b/Grow.c
> index 3322cf7..99807b4 100644
> --- a/Grow.c
> +++ b/Grow.c
> @@ -2058,12 +2058,17 @@ static int wait_backup(struct mdinfo *sra,
> sysfs_set_num(sra, NULL, "sync_max", offset + blocks + blocks2);
> if (offset == 0)
> sysfs_set_str(sra, NULL, "sync_action", "reshape");
> - do {
> +
> + if (sysfs_fd_get_ll(fd, &completed) < 0) {
> + close(fd);
> + return -1;
> + }
> + while (completed < offset + blocks) {
> char action[20];
> fd_set rfds;
> FD_ZERO(&rfds);
> FD_SET(fd, &rfds);
> - select(fd+1, NULL, NULL, &rfds, NULL);
> + select(fd+1, NULL, NULL, &rfds, &t);
> if (sysfs_fd_get_ll(fd, &completed) < 0) {
> close(fd);
> return -1;
> @@ -2072,7 +2077,7 @@ static int wait_backup(struct mdinfo *sra,
> action, 20) > 0 &&
> strncmp(action, "reshape", 7) != 0)
> break;
> - } while (completed < offset + blocks);
> + }
> close(fd);
>
> if (part) {
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 10/10] FIX: wait_backup() sometimes hungs
2010-12-03 7:45 ` Kwolek, Adam
@ 2010-12-03 10:35 ` Neil Brown
0 siblings, 0 replies; 20+ messages in thread
From: Neil Brown @ 2010-12-03 10:35 UTC (permalink / raw)
To: Kwolek, Adam
Cc: linux-raid@vger.kernel.org, Williams, Dan J, Ciechanowski, Ed
On Fri, 3 Dec 2010 07:45:14 +0000 "Kwolek, Adam" <adam.kwolek@intel.com>
wrote:
> I think you applied not a whole patch (no 't' definition and initialization), you missed:
>
> + struct timeval t;
> +
> + t.tv_sec = 1;
> + t.tv_usec = 0;
>
> so mdadm is not compiling
Thanks.
I meant to remove the 'struct timeval t', but I forgot to remove the 't' from
select.
And somehow I forgot to run 'make' after that patch.
Fixed now.
Thanks,
NeilBrown
>
> BR
> Adam
>
> > -----Original Message-----
> > From: Neil Brown [mailto:neilb@suse.de]
> > Sent: Friday, December 03, 2010 5:17 AM
> > To: Kwolek, Adam
> > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed
> > Subject: Re: [PATCH 10/10] FIX: wait_backup() sometimes hungs
> >
> > On Thu, 02 Dec 2010 09:19:58 +0100 Adam Kwolek <adam.kwolek@intel.com>
> > wrote:
> >
> > > Sometimes wait_backup() omits transition from reshape to iddle state
> > and mdadm seams to be hung.
> > > Add 1 sec. timeout wor waiting on select. This allows for wait_backup
> > exit when reshape is ended.
> > >
> > > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > > ---
> > >
> > > Grow.c | 6 +++++-
> > > 1 files changed, 5 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/Grow.c b/Grow.c
> > > index 24c5c39..e16b1ad 100644
> > > --- a/Grow.c
> > > +++ b/Grow.c
> > > @@ -2074,10 +2074,14 @@ static int wait_backup(struct mdinfo *sra,
> > > sysfs_set_str(sra, NULL, "sync_action", "reshape");
> > > do {
> > > char action[20];
> > > + struct timeval t;
> > > +
> > > + t.tv_sec = 1;
> > > + t.tv_usec = 0;
> > > fd_set rfds;
> > > FD_ZERO(&rfds);
> > > FD_SET(fd, &rfds);
> > > - select(fd+1, NULL, NULL, &rfds, NULL);
> > > + select(fd+1, NULL, NULL, &rfds, &t);
> > > if (sysfs_fd_get_ll(fd, &completed) < 0) {
> > > close(fd);
> > > return -1;
> >
> >
> > Thanks. However I don't think the 1 second timeout is necessary. This
> > is
> > really the same problem as the previous one. We just need to read
> > 'completed' before the first 'select'. Like this.
> >
> > Thanks,
> > NeilBrown
> >
> > commit 97bef35459306dfd291f40bc5221ad20ab9c21ba
> > Author: Adam Kwolek <adam.kwolek@intel.com>
> > Date: Fri Dec 3 15:15:51 2010 +1100
> >
> > FIX: wait_backup() sometimes hungs
> >
> > Sometimes wait_backup() omits transition from reshape to idle state
> > and mdadm seams to be hung. So check the 'complete' count
> > *before* waiting rather than only after.
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > Signed-off-by: NeilBrown <neilb@suse.de>
> >
> > diff --git a/Grow.c b/Grow.c
> > index 3322cf7..99807b4 100644
> > --- a/Grow.c
> > +++ b/Grow.c
> > @@ -2058,12 +2058,17 @@ static int wait_backup(struct mdinfo *sra,
> > sysfs_set_num(sra, NULL, "sync_max", offset + blocks + blocks2);
> > if (offset == 0)
> > sysfs_set_str(sra, NULL, "sync_action", "reshape");
> > - do {
> > +
> > + if (sysfs_fd_get_ll(fd, &completed) < 0) {
> > + close(fd);
> > + return -1;
> > + }
> > + while (completed < offset + blocks) {
> > char action[20];
> > fd_set rfds;
> > FD_ZERO(&rfds);
> > FD_SET(fd, &rfds);
> > - select(fd+1, NULL, NULL, &rfds, NULL);
> > + select(fd+1, NULL, NULL, &rfds, &t);
> > if (sysfs_fd_get_ll(fd, &completed) < 0) {
> > close(fd);
> > return -1;
> > @@ -2072,7 +2077,7 @@ static int wait_backup(struct mdinfo *sra,
> > action, 20) > 0 &&
> > strncmp(action, "reshape", 7) != 0)
> > break;
> > - } while (completed < offset + blocks);
> > + }
> > close(fd);
> >
> > if (part) {
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 00/10] Pre-migration patch series
2010-12-02 8:18 [PATCH 00/10] Pre-migration patch series Adam Kwolek
` (9 preceding siblings ...)
2010-12-02 8:19 ` [PATCH 10/10] FIX: wait_backup() sometimes hungs Adam Kwolek
@ 2010-12-03 4:19 ` Neil Brown
10 siblings, 0 replies; 20+ messages in thread
From: Neil Brown @ 2010-12-03 4:19 UTC (permalink / raw)
To: Adam Kwolek; +Cc: linux-raid, dan.j.williams, ed.ciechanowski
On Thu, 02 Dec 2010 09:18:41 +0100 Adam Kwolek <adam.kwolek@intel.com> wrote:
> This is bunch of patches that I've pull from my OLCE/Migration tree and I believe that
> they can be applied before we apply main feature (I'm curently going for some rework after your feedback).
>
> This series is for some behaviours of mdadm that I've forund so far.
> Mainly they are bugs that are present in code or behaviour that I've observe using takeover (i.e. geo map fix).
> Some of them are not visible at this moment, when we reshape big arrays (i.e. wait functions fixes) etc.
>
>
> ---
Thanks.
>
> Adam Kwolek (10):
> FIX: wait_backup() sometimes hungs
applied.
> FIX: Honor !reshape state on wait_reshape() entry
Applied.
> FIX: sync_completed_fd handler has to be closed
applied. Also closed metadata_version_fd.
> FIX: Do not use layout for raid4 and raid0 while geo map computing
applied.
> FIX: open backup file for reshape as function
applied
> Add spares to raid0 array using takeover
Not applied, as explained.
> Add support to skip slot configuration
Applied.
> FIX: Add error code for raid_disks set
Not applied. As Dan said, this doesn't make sense in the current code.
> FIX: Problem with removing array after takeover
Not applied as explained.
> FIX: Cannot exit monitor after takeover
Applied.
Thanks,
NeilBrown
>
>
> Grow.c | 137 +++++++++++++++++++++++++++++++++++++----------------------
> Manage.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> managemon.c | 1
> mdadm.h | 8 +++
> monitor.c | 16 +++++++
> msg.c | 8 +++
> restripe.c | 5 ++
> sysfs.c | 3 +
> 8 files changed, 242 insertions(+), 56 deletions(-)
>
^ permalink raw reply [flat|nested] 20+ messages in thread