Multipath: 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 in 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. [Mike Christie] --- diff/drivers/md/dm-mpath.c 2004-04-06 15:54:30.809823016 +0100 +++ source/drivers/md/dm-mpath.c 2004-04-06 15:54:32.713533608 +0100 @@ -19,9 +19,6 @@ #include #include -/* 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/drivers/md/dm-path-selector.c 2004-04-06 15:54:30.809823016 +0100 +++ source/drivers/md/dm-path-selector.c 2004-04-06 15:54:32.713533608 +0100 @@ -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/drivers/md/dm-path-selector.h 2004-04-06 15:54:30.810822864 +0100 +++ source/drivers/md/dm-path-selector.h 2004-04-06 15:54:32.714533456 +0100 @@ -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; };