From: Milan Broz <mbroz@redhat.com>
Date: Tue, 14 Oct 2008 10:22:40 +0200
Subject: [dm] mpath: moving path activation to workqueue panics
Message-id: 48F456D0.4060308@redhat.com
O-Subject: [RHEL 5.3 PATCH] dm mpath: moving path activation to workqueue causes a panic
Bugzilla: 465570
RH-Acked-by: Alasdair G Kergon <agk@redhat.com>

RHEL5.3 dm mpath: moving path activation to workqueue causes a panic
Resolves: rhbz#465570
Patch is upstream, commit 7253a33434245ee8243897559188186df65f3611

Moving the path activation to workqueue along with scsi_dh patches introduced
a race. It is due to the fact that the current_pgpath (in the multipath data
structure) can be modified if changes happen in any of the paths leading to
the lun. If the changes lead to current_pgpath being set to NULL, then it
leads to the invalid access which results in the panic.

This patch fixes that by storing the pgpath to activate in the multipath data
structure and properly protecting it.

Note that if activate_path is called twice in succession with different pgpath,
with the second one being called before the first one is done, then activate
path will be called twice for the second pgpath, which is fine.

Patch provided and tested by customer.
Also test build and basic testing passed.

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index b96d7c3..f09d272 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -87,6 +87,7 @@ struct multipath {
 
 	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
@@ -151,6 +152,7 @@ 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;
 
@@ -159,6 +161,10 @@ 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);
 	}
 }
@@ -425,6 +431,7 @@ static void process_queued_ios(void *data)
 		__choose_pgpath(m);
 
 	pgpath = m->current_pgpath;
+	m->pgpath_to_activate = m->current_pgpath;
 
 	if ((pgpath && !m->queue_io) ||
 	    (!pgpath && !m->queue_if_no_path))
@@ -1174,7 +1181,15 @@ static void activate_path(void *data)
 {
 	int ret;
 	struct multipath *m = (struct multipath *) data;
-	struct path *path = &m->current_pgpath->path;
+	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;
 
 	ret = scsi_dh_activate(bdev_get_queue(path->dev->bdev));
 	pg_init_done(path, ret);