From: Jesse Larrew <jlarrew@redhat.com>
Date: Thu, 14 May 2009 01:00:22 -0400
Subject: [md] handle multiple paths in pg_init
Message-id: 20090514045731.10947.82027.sendpatchset@squad5-lp1.lab.bos.redhat.com
O-Subject: [PATCH RHEL5.4 9/10 BZ489582] Handle multiple paths in pg_init()
Bugzilla: 489582
RH-Acked-by: Mike Christie <mchristi@redhat.com>

RHBZ#:
======
https://bugzilla.redhat.com/show_bug.cgi?id=489582

Description:
===========
This is a bug fix for all archs.

This patch fixes a problem while reinstating passive paths.

There is a problem which was caused due to the architectural change made
when we moved from dm hardware handler to SCSI hardware handler.

In dm, handler was called to do a pg_init for a path group, and there
was no state maintained in hardware handler code for each path.

In SCSI dh, "state" is maintanined per path, as we wanted to fail I/O
early on that path if it is not the active path.

All of the hardware handlers do have a state now, and they are set to
active and (some form of) inactive. All of them have prep_fn(), which
use this "state" to fail the I/O without it ever being sent to the
device.

So, in effect when dm-multipath calls scsi_dh_activate(), activate is
sent to only one path and the "state" of that path is changed
appropriately to "active" while other paths in the same path group are
never changed as they never got an "activate".

In order make sure all the paths in a path group get their state set
properly when a pg_init happens, we need to call scsi_dh_activate() on
all paths in a path group.

Doing this at the hardware handler layer is not a good option as we
want the multipath layer to define the relationship between path and
path groups and not the hardware handler.

The attached patch sends an "activate" on each path in a path group when
a path group is switched. It also sends an activate when a path is
reinstated.

RHEL Version Found:
================
RHEL 5.3

kABI Status:
============
No symbols were harmed.

Brew:
=====
Built on all platforms.
http://brewweb.devel.redhat.com/brew/taskinfo?taskID=1794596

Upstream Status:
================
http://patchwork.kernel.org/patch/20520/

Test Status:
============
This patch has been tested by Chandra Seetharaman at IBM
(sekharan@us.ibm.com).

Testing methodology:

Have 2 HBAs connected to different switches and both the switches
connected to both the controllers. In effect, get 4 paths established.
So we will see 2 path groups with 2 paths each, each path group
corresponds to a single controller.

Knock off a controller. The path group will switch over to the passive
controller and io will continue.

Bring the controller back. Path group will switch over to the preferred
controller and both the paths will become active.

Without this patch only one path will become active and the other path
will stay in a failed state.

===============================================================

Jesse Larrew
IBM Onsite Partner
978-392-3183
jlarrew@redhat.com

Proposed Patch:
===============
This patch is based on 2.6.18-136.el5.

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 6cac8a2..ef42277 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -34,6 +34,7 @@ struct pgpath {
 	unsigned fail_count;		/* Cumulative failure count */
 
 	struct path path;
+	struct work_struct activate_path;
 };
 
 #define path_to_pgpath(__pgp) container_of((__pgp), struct pgpath, path)
@@ -86,8 +87,6 @@ struct multipath {
 	unsigned queue_size;
 
 	struct work_struct trigger_event;
-	struct work_struct activate_path;
-	struct pgpath *pgpath_to_activate;
 
 	/*
 	 * We must use a mempool of mpath_io structs so that we
@@ -126,6 +125,7 @@ static struct pgpath *alloc_pgpath(void)
 	if (pgpath) {
 		memset(pgpath, 0, sizeof(*pgpath));
 		pgpath->path.is_active = 1;
+		INIT_WORK(&pgpath->activate_path, activate_path, pgpath);
 	}
 
 	return pgpath;
@@ -152,7 +152,6 @@ static struct priority_group *alloc_priority_group(void)
 
 static void free_pgpaths(struct list_head *pgpaths, struct dm_target *ti)
 {
-	unsigned long flags;
 	struct pgpath *pgpath, *tmp;
 	struct multipath *m = (struct multipath *) ti->private;
 
@@ -161,10 +160,6 @@ static void free_pgpaths(struct list_head *pgpaths, struct dm_target *ti)
 		if (m->hw_handler_name)
 			scsi_dh_detach(bdev_get_queue(pgpath->path.dev->bdev));
 		dm_put_device(ti, pgpath->path.dev);
-		spin_lock_irqsave(&m->lock, flags);
-		if (m->pgpath_to_activate == pgpath)
-			m->pgpath_to_activate = NULL;
-		spin_unlock_irqrestore(&m->lock, flags);
 		free_pgpath(pgpath);
 	}
 }
@@ -195,7 +190,6 @@ static struct multipath *alloc_multipath(void)
 		m->queue_io = 1;
 		INIT_WORK(&m->process_queued_ios, process_queued_ios, m);
 		INIT_WORK(&m->trigger_event, trigger_event, m);
-		INIT_WORK(&m->activate_path, activate_path, m);
 		m->mpio_pool = mempool_create_slab_pool(MIN_IOS, _mpio_cache);
 		if (!m->mpio_pool) {
 			kfree(m);
@@ -418,7 +412,7 @@ static void process_queued_ios(void *data)
 {
 	struct multipath *m = (struct multipath *) data;
 	struct hw_handler *hwh = &m->hw_handler;
-	struct pgpath *pgpath = NULL;
+	struct pgpath *pgpath = NULL, *tmp;
 	unsigned init_required = 0, must_queue = 1;
 	unsigned long flags;
 
@@ -437,23 +431,24 @@ static void process_queued_ios(void *data)
 		must_queue = 0;
 
 	if (m->pg_init_required && !m->pg_init_in_progress && pgpath) {
-		m->pgpath_to_activate = pgpath;
 		m->pg_init_count++;
 		m->pg_init_required = 0;
-		m->pg_init_in_progress = 1;
-		init_required = 1;
+		if (hwh->type) {
+			m->pg_init_in_progress = 1;
+			init_required = 1;
+		} else {
+	 		list_for_each_entry(tmp, &pgpath->pg->pgpaths, list) {
+ 				queue_work(kmpath_handlerd, &tmp->activate_path);
+ 				m->pg_init_in_progress++;
+ 			}
+		}
 	}
 
 out:
 	spin_unlock_irqrestore(&m->lock, flags);
 
-	if (init_required) {
-		if (hwh->type)
-			hwh->type->pg_init(hwh, pgpath->pg->bypassed,
-							 &pgpath->path);
-		else
-			queue_work(kmpath_handlerd, &m->activate_path);
-	}
+	if (init_required)
+		hwh->type->pg_init(hwh, pgpath->pg->bypassed, &pgpath->path);
 
 	if (!must_queue)
 		dispatch_queued_ios(m);
@@ -951,9 +946,13 @@ static int reinstate_path(struct pgpath *pgpath)
 
 	pgpath->path.is_active = 1;
 
-	m->current_pgpath = NULL;
-	if (!m->nr_valid_paths++ && m->queue_size)
+	if (!m->nr_valid_paths++ && m->queue_size) {
+		m->current_pgpath = NULL;
 		queue_work(kmultipathd, &m->process_queued_ios);
+	} else if (m->hw_handler_name && (m->current_pg == pgpath->pg)) {
+ 		m->pg_init_in_progress++;
+		queue_work(kmpath_handlerd, &pgpath->activate_path);
+	}
 
 	dm_path_uevent(DM_UEVENT_PATH_REINSTATED, m->ti,
 		      pgpath->path.dev->name, m->nr_valid_paths);
@@ -1167,35 +1166,29 @@ static void pg_init_done(struct path *path, int errors)
 
 	spin_lock_irqsave(&m->lock, flags);
 	if (errors) {
-		DMERR("Could not failover device. Error %d.", errors);
-		m->current_pgpath = NULL;
-		m->current_pg = NULL;
+		if (pgpath == m->current_pgpath) {
+			DMERR("Could not failover device. Error %d.", errors);
+			m->current_pgpath = NULL;
+			m->current_pg = NULL;
+		}
 	} else if (!m->pg_init_required) {
 		m->queue_io = 0;
 		pg->bypassed = 0;
 	}
 
-	m->pg_init_in_progress = 0;
-	queue_work(kmultipathd, &m->process_queued_ios);
+	m->pg_init_in_progress--;
+	if (!m->pg_init_in_progress)
+		queue_work(kmultipathd, &m->process_queued_ios);
 	spin_unlock_irqrestore(&m->lock, flags);
 }
 
 static void activate_path(void *data)
 {
 	int ret;
-	struct multipath *m = (struct multipath *) data;
-	struct path *path;
-	unsigned long flags;
-
-	spin_lock_irqsave(&m->lock, flags);
-	path = &m->pgpath_to_activate->path;
-	m->pgpath_to_activate = NULL;
-	spin_unlock_irqrestore(&m->lock, flags);
-	if (!path)
-		return;
+	struct pgpath *pgpath = (struct pgpath *) data;
 
-	ret = scsi_dh_activate(bdev_get_queue(path->dev->bdev));
-	pg_init_done(path, ret);
+	ret = scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev));
+	pg_init_done(&pgpath->path, ret);
 }
 
 /*