Reviewed and Tested-by: Babu Moger <babu.moger@xxxxxxx> > -----Original Message----- > From: Chandra Seetharaman [mailto:sekharan@xxxxxxxxxx] > Sent: Monday, March 30, 2009 1:08 PM > To: dm-devel > Cc: Alasdair G Kergon; Moger, Babu; Hannes Reinecke; Mike Anderson > Subject: [PATCH] Handle multipath paths in a path group properly during > pg_init > > Resending the patch to get in patchwork... > ------- > The problem reported by Moger Babu was caused due to the architectural > change made when we moved from dm hardware handler to SCSI hardware > handler. > > Thanks Babu for finding and reporting the bug. > > 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. > > As Babu has noted in his email, the pg_init/activate is sent on 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". > > Attached is a patch (compiled, tested, but not clean yet), which makes > changes in the dm-multipath layer to send an "activate" on each paths in > the path groups. > > Mike (Anderson) and I had a discussion about whether to implement this > in the dm-mulitpath layer or in the SCSI hardware handler layer and we > came to a conclusion that it is best suited to be in the dm-mulitpath > layer as it is the one that knows the relationship between different > paths. > > If it were to be done at the Hardware handler layer, then the hardware > handler may end up having a different notion of the path relationship > and hence may not work as expected by the dm-multipath layer. > > This patch has been tested by Hannes in EMC storage. Babu and I tested it > in LSI storage. > ---------- > > Signed-off-by: Chandra Seetharaman <sekharan@xxxxxxxxxx> > > --- > Index: linux-2.6.29/drivers/md/dm-mpath.c > =================================================================== > --- linux-2.6.29.orig/drivers/md/dm-mpath.c > +++ linux-2.6.29/drivers/md/dm-mpath.c > @@ -36,6 +36,7 @@ struct pgpath { > > struct dm_path path; > struct work_struct deactivate_path; > + struct work_struct activate_path; > }; > > #define path_to_pgpath(__pgp) container_of((__pgp), struct pgpath, path) > @@ -65,8 +66,6 @@ struct multipath { > spinlock_t lock; > > const char *hw_handler_name; > - struct work_struct activate_path; > - struct pgpath *pgpath_to_activate; > unsigned nr_priority_groups; > struct list_head priority_groups; > unsigned pg_init_required; /* pg_init needs calling? */ > @@ -129,6 +128,7 @@ static struct pgpath *alloc_pgpath(void) > if (pgpath) { > pgpath->is_active = 1; > INIT_WORK(&pgpath->deactivate_path, deactivate_path); > + INIT_WORK(&pgpath->activate_path, activate_path); > } > > return pgpath; > @@ -170,10 +170,6 @@ 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); > - 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); > } > } > @@ -203,7 +199,6 @@ static struct multipath *alloc_multipath > m->queue_io = 1; > INIT_WORK(&m->process_queued_ios, process_queued_ios); > INIT_WORK(&m->trigger_event, trigger_event); > - INIT_WORK(&m->activate_path, activate_path); > m->mpio_pool = mempool_create_slab_pool(MIN_IOS, _mpio_cache); > if (!m->mpio_pool) { > kfree(m); > @@ -428,8 +423,8 @@ static void process_queued_ios(struct wo > { > struct multipath *m = > container_of(work, struct multipath, process_queued_ios); > - struct pgpath *pgpath = NULL; > - unsigned init_required = 0, must_queue = 1; > + struct pgpath *pgpath = NULL, *tmp; > + unsigned must_queue = 1; > unsigned long flags; > > spin_lock_irqsave(&m->lock, flags); > @@ -447,19 +442,15 @@ static void process_queued_ios(struct wo > 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; > + 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) > - queue_work(kmpath_handlerd, &m->activate_path); > - > if (!must_queue) > dispatch_queued_ios(m); > } > @@ -1111,27 +1102,20 @@ static void pg_init_done(struct dm_path > 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(struct work_struct *work) > { > int ret; > - struct multipath *m = > - container_of(work, struct multipath, activate_path); > - struct dm_path *path; > - unsigned long flags; > + struct pgpath *pgpath = > + container_of(work, struct pgpath, activate_path); > > - 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); > + ret = scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev)); > + pg_init_done(&pgpath->path, ret); > } > > /* > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
