Attachment 316641 Details for Bug 438761 – Fix a race situation

Fix a race situation

fix_race_issue_with_activation_rhel (text/plain), 3.02 KB, created by IBM Bug Proxy on 2008-09-13 01:57:51 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: IBM Bug Proxy

Created: 2008-09-13 01:57:51 UTC

Size: 3.02 KB

patch

obsolete

>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 below.
>
>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.
>
>Signed-off-by: Chandra Seetharaman <sekharan@us.ibm.com>
>--------------------
>Unable to handle kernel paging request for data at address 0x00000028
>Faulting instruction address: 0xd000000000aa1f54
>cpu 0x6: Vector: 300 (Data Access) at [c000000071327a30]
>    pc: d000000000aa1f54: .activate_path+0x30/0x1ec [dm_multipath]
>    lr: c000000000081534: .run_workqueue+0xdc/0x168
>    sp: c000000071327cb0
>   msr: 8000000000009032
>   dar: 28
> dsisr: 40000000
>  current = 0xc0000000e3e62370
>  paca    = 0xc0000000004d5a80
>    pid   = 2087, comm = kmpath_handlerd
>enter ? for help
>6:mon> 
>--------------------
>Index: linux-2.6.18.ppc64/drivers/md/dm-mpath.c
>===================================================================
>--- linux-2.6.18.ppc64.orig/drivers/md/dm-mpath.c
>+++ linux-2.6.18.ppc64/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_prio
> 
> 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,11 @@ static void free_pgpaths(struct list_hea
> 		if (m->hw_handler_name)
> 			scsi_dh_detach(bdev_get_queue(pgpath->path.dev->bdev));
> 		dm_put_device(ti, pgpath->path.dev);
>+		if (m->pgpath_to_activate == pgpath) {
>+			spin_lock_irqsave(&m->lock, flags);
>+			m->pgpath_to_activate = NULL;
>+			spin_unlock_irqrestore(&m->lock, flags);
>+		}
> 		free_pgpath(pgpath);
> 	}
> }
>@@ -425,6 +432,7 @@ static void process_queued_ios(void *dat
> 		__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))
>@@ -1167,7 +1175,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);

Actions: View

Attachments on bug 438761: 314153 | 314154 | 314155 | 314156 | 314157 | 314158 | 314159 | 314160 | 314161 | 314162 | 314163 | 314339 | 314426 | 314828 | 314829 | 316641