[dm-devel] [PATCH RFC 1/5] add support for manual failover devices

[Mike Christie <michaelc cs wisc edu>]

01-ps-failpath.patch - move the path failure decision to the path
selector. Some ps's may want to select paths that have a couple
failures over one that has N failures. If a ps has to track this info
there didn't seem to be a compelling reason to also do it in
dm-mpath.c. Well code duplication and sharing is a good reason to do
it at dm-mpath. It would be nice if I could just add an init function
to any ps and leach off other people's work :) but is that too modular?

Additionally, if a ps fails to activate a path or cannot use it
during initialization it may want to fail it.

diffstat
 dm-mpath.c         |   71 ++++++++++++-----------------------------------------
 dm-path-selector.c |   50 +++++++++++++++++++++++++++++--------
 dm-path-selector.h |    8 ++---
 3 files changed, 61 insertions(+), 68 deletions(-)

diff -aurp linux-2.6.5-rc1-udm1-orig/drivers/md/dm-mpath.c linux-2.6.5-rc1-udm1-work/drivers/md/dm-mpath.c
--- linux-2.6.5-rc1-udm1-orig/drivers/md/dm-mpath.c	2004-04-02 01:24:18.000000000 -0800
+++ linux-2.6.5-rc1-udm1-work/drivers/md/dm-mpath.c	2004-04-03 22:48:45.116161233 -0800
@@ -19,9 +19,6 @@
 #include <linux/workqueue.h>
 #include <asm/atomic.h>
 
-/* FIXME: get rid of this */
-#define MPATH_FAIL_COUNT	1
-
 /*
  * We don't want to call the path selector for every single io
  * that comes through, so instead we only consider changing paths
@@ -42,10 +39,6 @@ struct path {
 
 	struct dm_dev *dev;
 	struct priority_group *pg;
-
-	spinlock_t failed_lock;
-	int has_failed;
-	unsigned fail_count;
 };
 
 struct priority_group {
@@ -67,7 +60,6 @@ struct multipath {
 	struct list_head priority_groups;
 
 	spinlock_t lock;
-	unsigned nr_valid_paths;
 
 	struct path *current_path;
 	unsigned current_count;
@@ -99,11 +91,8 @@ static struct path *alloc_path(void)
 {
 	struct path *path = kmalloc(sizeof(*path), GFP_KERNEL);
 
-	if (path) {
+	if (path)
 		memset(path, 0, sizeof(*path));
-		path->failed_lock = SPIN_LOCK_UNLOCKED;
-		path->fail_count = MPATH_FAIL_COUNT;
-	}
 
 	return path;
 }
@@ -204,13 +193,11 @@ static int __choose_path(struct multipat
 	struct priority_group *pg;
 	struct path *path = NULL;
 
-	if (m->nr_valid_paths) {
-		/* loop through the priority groups until we find a valid path. */
-		list_for_each_entry (pg, &m->priority_groups, list) {
-			path = pg->ps->type->select_path(pg->ps);
-			if (path)
-				break;
-		}
+	/* loop through the priority groups until we find a valid path. */
+	list_for_each_entry (pg, &m->priority_groups, list) {
+		path = pg->ps->type->select_path(pg->ps);
+		if (path)
+			break;
 	}
 
 	m->current_path = path;
@@ -471,7 +458,6 @@ static int multipath_ctr(struct dm_targe
 		if (!pg)
 			goto bad;
 
-		m->nr_valid_paths += pg->nr_paths;
 		list_add_tail(&pg->list, &m->priority_groups);
 	}
 
@@ -512,31 +498,19 @@ static int multipath_map(struct dm_targe
 	return 1;
 }
 
-static void fail_path(struct path *path)
+static void update_path(struct path *path, int error)
 {
 	unsigned long flags;
-	struct multipath *m;
-
-	spin_lock_irqsave(&path->failed_lock, flags);
+	struct multipath *m = path->pg->m;
 
-	/* FIXME: path->fail_count is brain dead */
-	if (!path->has_failed && !--path->fail_count) {
-		m = path->pg->m;
-
-		path->has_failed = 1;
-		path->pg->ps->type->fail_path(path->pg->ps, path);
-		schedule_work(&m->trigger_event);
+	path->pg->ps->type->update_path(path->pg->ps, path, error);
 
-		spin_lock(&m->lock);
-		m->nr_valid_paths--;
-
-		if (path == m->current_path)
-			m->current_path = NULL;
-
-		spin_unlock(&m->lock);
-	}
+	spin_lock_irqsave(&m->lock, flags);
+	if (path == m->current_path)
+		m->current_path = NULL;
+	spin_unlock_irqrestore(&m->lock, flags);
 
-	spin_unlock_irqrestore(&path->failed_lock, flags);
+	schedule_work(&m->trigger_event);
 }
 
 static int do_end_io(struct multipath *m, struct bio *bio,
@@ -545,14 +519,7 @@ static int do_end_io(struct multipath *m
 	int r;
 
 	if (error) {
-		spin_lock(&m->lock);
-		if (!m->nr_valid_paths) {
-			spin_unlock(&m->lock);
-			return -EIO;
-		}
-		spin_unlock(&m->lock);
-
-		fail_path(io->path);
+		update_path(io->path, error);
 
 		/* remap */
 		dm_bio_restore(&io->details, bio);
@@ -598,7 +565,6 @@ static int multipath_status(struct dm_ta
 			    char *result, unsigned int maxlen)
 {
 	int sz = 0;
-	unsigned long flags;
 	struct multipath *m = (struct multipath *) ti->private;
 	struct priority_group *pg;
 	struct path *p;
@@ -616,12 +582,9 @@ static int multipath_status(struct dm_ta
 
 			list_for_each_entry(p, &pg->paths, list) {
 				format_dev_t(buffer, p->dev->bdev->bd_dev);
-				spin_lock_irqsave(&p->failed_lock, flags);
-				EMIT("%s %s %u ", buffer,
-				     p->has_failed ? "F" : "A", p->fail_count);
-				pg->ps->type->status(pg->ps, p, type,
+				EMIT("%s ", buffer);
+				sz += pg->ps->type->status(pg->ps, p, type,
 						     result + sz, maxlen - sz);
-				spin_unlock_irqrestore(&p->failed_lock, flags);
 			}
 		}
 		break;
diff -aurp linux-2.6.5-rc1-udm1-orig/drivers/md/dm-path-selector.c linux-2.6.5-rc1-udm1-work/drivers/md/dm-path-selector.c
--- linux-2.6.5-rc1-udm1-orig/drivers/md/dm-path-selector.c	2004-04-02 01:24:18.000000000 -0800
+++ linux-2.6.5-rc1-udm1-work/drivers/md/dm-path-selector.c	2004-04-03 22:48:53.190046847 -0800
@@ -127,6 +127,8 @@ int dm_unregister_path_selector(struct p
 struct path_info {
 	struct list_head list;
 	struct path *path;
+
+	unsigned fail_count;
 };
 
 static struct path_info *path_lookup(struct list_head *head, struct path *p)
@@ -143,6 +145,10 @@ static struct path_info *path_lookup(str
 /*-----------------------------------------------------------------
  * Round robin selector
  *---------------------------------------------------------------*/
+
+/* FIXME: get rid of this */
+#define MPATH_FAIL_COUNT	1
+
 struct selector {
 	spinlock_t lock;
 
@@ -215,6 +221,7 @@ static int rr_add_path(struct path_selec
 		return -ENOMEM;
 	}
 
+	pi->fail_count = MPATH_FAIL_COUNT;
 	pi->path = path;
 
 	spin_lock(&s->lock);
@@ -224,7 +231,8 @@ static int rr_add_path(struct path_selec
 	return 0;
 }
 
-static void rr_fail_path(struct path_selector *ps, struct path *p)
+static void rr_update_path(struct path_selector *ps, struct path *p,
+			   int error)
 {
 	unsigned long flags;
 	struct selector *s = (struct selector *) ps->context;
@@ -235,14 +243,9 @@ static void rr_fail_path(struct path_sel
 	 * mind the expense of these searches.
 	 */
 	spin_lock_irqsave(&s->lock, flags);
-	pi = path_lookup(&s->valid_paths, p);
-	if (!pi)
-		pi = path_lookup(&s->invalid_paths, p);
-
-	if (!pi)
-		DMWARN("asked to change the state of an unknown path");
 
-	else
+	pi = path_lookup(&s->valid_paths, p);
+	if (pi && !--pi->fail_count) 
 		list_move(&pi->list, &s->invalid_paths);
 
 	spin_unlock_irqrestore(&s->lock, flags);
@@ -269,7 +272,34 @@ static struct path *rr_select_path(struc
 static int rr_status(struct path_selector *ps, struct path *path,
 		     status_type_t type, char *result, unsigned int maxlen)
 {
-	return 0;
+	unsigned long flags;
+	struct path_info *pi;
+	int failed = 0;
+	struct selector *s = (struct selector *) ps->context;
+	int sz = 0;
+
+	if (type == STATUSTYPE_TABLE)
+		return 0;
+
+	spin_lock_irqsave(&s->lock, flags);
+
+	/*
+	 * Is status called often for testing or something?
+	 * If so maybe a ps's info should be allocated w/ path
+	 * so a simple container_of can be used.
+	 */
+	pi = path_lookup(&s->valid_paths, path);
+	if (!pi) {
+		failed = 1;
+		pi = path_lookup(&s->invalid_paths, path);
+	}
+
+	sz = scnprintf(result, maxlen, "%s %u ", failed ? "F" : "A",
+		       pi->fail_count);
+
+	spin_unlock_irqrestore(&s->lock, flags);
+
+	return sz;
 }
 
 static struct path_selector_type rr_ps = {
@@ -279,7 +309,7 @@ static struct path_selector_type rr_ps =
 	.ctr = rr_ctr,
 	.dtr = rr_dtr,
 	.add_path = rr_add_path,
-	.fail_path = rr_fail_path,
+	.update_path = rr_update_path,
 	.select_path = rr_select_path,
 	.status = rr_status,
 };
diff -aurp linux-2.6.5-rc1-udm1-orig/drivers/md/dm-path-selector.h linux-2.6.5-rc1-udm1-work/drivers/md/dm-path-selector.h
--- linux-2.6.5-rc1-udm1-orig/drivers/md/dm-path-selector.h	2004-04-02 01:24:18.000000000 -0800
+++ linux-2.6.5-rc1-udm1-work/drivers/md/dm-path-selector.h	2004-04-03 22:55:02.669433656 -0800
@@ -52,10 +52,10 @@ typedef	int (*ps_add_path_fn) (struct pa
 typedef	struct path *(*ps_select_path_fn) (struct path_selector *ps);
 
 /*
- * Notify the selector that a path has failed.
+ * Notify the selector that an IO failure has occured.
  */
-typedef	void (*ps_fail_path_fn) (struct path_selector *ps,
-				 struct path *p);
+typedef	void (*ps_update_path_fn) (struct path_selector *ps,
+				   struct path *p, int error);
 
 /*
  * Table content based on parameters added in ps_add_path_fn
@@ -75,7 +75,7 @@ struct path_selector_type {
 	ps_dtr_fn dtr;
 
 	ps_add_path_fn add_path;
-	ps_fail_path_fn fail_path;
+	ps_update_path_fn update_path;
 	ps_select_path_fn select_path;
 	ps_status_fn status;
 };