[PATCH 2/2] Monitor: don't add more spares than needed

[PATCH 2/2] Monitor: don't add more spares than needed

[Czarnowska, Anna]

From ce59ecdb2b6a6e1fc65c645d0ca8733056e4979a Mon Sep 17 00:00:00 2001
From: Anna Czarnowska <anna.czarnowska@intel.com>
Date: Thu, 2 Dec 2010 22:48:19 +0100
Subject: [PATCH 2/2] Monitor: don't add more spares than needed
Cc: linux-raid@vger.kernel.org, Williams, Dan J <dan.j.williams@intel.com>, Ciechanowski, Ed <ed.ciechanowski@intel.com>

When we add a spare to a container it takes a while
before it is noticed by mdmon and recovery starts.
During this time the array remains degraded but we don't want to add
any more spares to this container. Therefore we must check container
with degraded array if it doesn't already have a suitable spare.
container_choose_spare is reused with from=to
Domain check is not needed in this situation.

Ping_manager after moving disk is needed to be able to see
newly added disk in container after coming back through the loop.

Signed-off-by: Anna Czarnowska <anna.czarnowska@intel.com>
---
 Monitor.c |   33 +++++++++++++++++++++++----------
 1 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/Monitor.c b/Monitor.c
index ab6eb2d..c75b2c1 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -743,6 +743,8 @@ static int move_spare(struct state *from, struct state *to,
 		devlist.disposition = 'a';
 		if (Manage_subdevs(to->devname, fd1, &devlist, -1, 0) == 0) {
 			alert("MoveSpare", to->devname, from->devname, info);
+			ping_manager(to->devname);
+			ping_manager(from->devname);
 			close(fd1);
 			close(fd2);
 			return 1;
@@ -842,7 +844,6 @@ static dev_t container_choose_spare(struct state *from, struct state *to,
 	
 	for (d = disks->devs ; d && !dev ; d = d->next) {
 		if (d->disk.state == 0) {
-			struct dev_policy *pol;
 			unsigned long long dev_size;
 			dev = makedev(d->disk.major,d->disk.minor);
 			
@@ -852,14 +853,17 @@ static dev_t container_choose_spare(struct state *from, struct state *to,
 				dev = 0;
 				continue;
 			}
-			pol = devnum_policy(dev);
-			if (from->spare_group)
-				pol_add(&pol, pol_domain,
-					from->spare_group, NULL);
-			if (!domain_test(domlist, pol, to->metadata->ss->name))
-				dev = 0;
-			
-			dev_policy_free(pol);
+			if (from != to) {
+				struct dev_policy *pol
+					= devnum_policy(dev);
+				if (from->spare_group)
+					pol_add(&pol, pol_domain,
+						from->spare_group, NULL);
+				if (!domain_test(domlist, pol,
+						 to->metadata->ss->name))
+					dev = 0;
+				dev_policy_free(pol);
+			}
 		}
 	}
 	sysfs_free(disks);
@@ -880,12 +884,22 @@ static void try_spare_migration(struct state *statelist, struct alert_info *info
 			int d;
 			struct state *to = st;
 			unsigned long long min_size;
+			dev_t devid;
 			
 			if (to->parent)
 				/* member of a container */
 				to = to->parent;
 
 			min_size = min_spare_size_required(to);
+			if (to->metadata->ss->external) {
+				/* We must make sure there is
+				 * no suitable spare in container already.
+				 * If there is we don't add more */
+				devid = container_choose_spare(
+					to, to, NULL, min_size);
+				if (devid > 0)
+					continue;
+			}
 			for (d = 0; d < MaxDisks; d++)
 				if (to->devid[d])
 					domainlist_add_dev(&domlist,
@@ -895,7 +909,6 @@ static void try_spare_migration(struct state *statelist, struct alert_info *info
 				domain_add(&domlist, to->spare_group);
 
 			for (from=statelist ; from ; from=from->next) {
-				dev_t devid;
 				if (!check_donor(from, to, domlist))
 					continue;
 				if (from->metadata->ss->external)
-- 
1.7.1

Re: [PATCH 2/2] Monitor: don't add more spares than needed

[Neil Brown]

On Thu, 2 Dec 2010 21:59:27 +0000 "Czarnowska, Anna"
<anna.czarnowska@intel.com> wrote:

> >From ce59ecdb2b6a6e1fc65c645d0ca8733056e4979a Mon Sep 17 00:00:00 2001
> From: Anna Czarnowska <anna.czarnowska@intel.com>
> Date: Thu, 2 Dec 2010 22:48:19 +0100
> Subject: [PATCH 2/2] Monitor: don't add more spares than needed
> Cc: linux-raid@vger.kernel.org, Williams, Dan J <dan.j.williams@intel.com>, Ciechanowski, Ed <ed.ciechanowski@intel.com>
> 
> When we add a spare to a container it takes a while
> before it is noticed by mdmon and recovery starts.
> During this time the array remains degraded but we don't want to add
> any more spares to this container. Therefore we must check container
> with degraded array if it doesn't already have a suitable spare.
> container_choose_spare is reused with from=to
> Domain check is not needed in this situation.
> 
> Ping_manager after moving disk is needed to be able to see
> newly added disk in container after coming back through the loop.
> 

Thanks for these.  I have applied both.

The second I made a few little changes to resulting the the following.

thanks,
NeilBrown


commit e9a2ac028eb66fdc50c079fc9bf03aa4325bcd5f
Author: Anna Czarnowska <anna.czarnowska@intel.com>
Date:   Fri Dec 3 14:11:29 2010 +1100

    Monitor: don't add more spares than needed
    
    When we add a spare to a container it takes a while
    before it is noticed by mdmon and recovery starts.
    During this time the array remains degraded but we don't want to add
    any more spares to this container. Therefore we must check container
    with degraded array if it doesn't already have a suitable spare.
    container_choose_spare is reused with from=to
    Domain check is not needed in this situation.
    
    Ping_manager after moving disk is needed to be able to see
    newly added disk in container after coming back through the loop.
    
    Signed-off-by: Anna Czarnowska <anna.czarnowska@intel.com>
    Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/Monitor.c b/Monitor.c
index 8f1bfea..61550ee 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -743,6 +743,11 @@ static int move_spare(struct state *from, struct state *to,
 		devlist.disposition = 'a';
 		if (Manage_subdevs(to->devname, fd1, &devlist, -1, 0) == 0) {
 			alert("MoveSpare", to->devname, from->devname, info);
+			/* make sure we will see newly added spare before next
+			 * time through loop
+			 */
+			ping_manager(to->devname);
+			ping_manager(from->devname);
 			close(fd1);
 			close(fd2);
 			return 1;
@@ -853,13 +858,20 @@ static dev_t container_choose_spare(struct state *from, struct state *to,
 				dev = 0;
 				continue;
 			}
+			if (from == to)
+				/* Just checking if destination already has
+				 * a spare, no need to check policy, we are
+				 * done.
+				 */
+				break;
+
 			pol = devnum_policy(dev);
 			if (from->spare_group)
 				pol_add(&pol, pol_domain,
 					from->spare_group, NULL);
 			if (!domain_test(domlist, pol, to->metadata->ss->name))
 				dev = 0;
-			
+
 			dev_policy_free(pol);
 		}
 	}
@@ -887,6 +899,15 @@ static void try_spare_migration(struct state *statelist, struct alert_info *info
 				to = to->parent;
 
 			min_size = min_spare_size_required(to);
+			if (to->metadata->ss->external) {
+				/* We must make sure there is
+				 * no suitable spare in container already.
+				 * If there is we don't add more */
+				dev_t devid = container_choose_spare(
+					to, to, NULL, min_size);
+				if (devid > 0)
+					continue;
+			}
 			for (d = 0; d < MaxDisks; d++)
 				if (to->devid[d])
 					domainlist_add_dev(&domlist,