On 02/28/2014 09:49 AM, Junichi Nomura wrote: > On 02/27/14 16:30, Hannes Reinecke wrote: >> activate_path() is run without a lock, so the path might be >> set to failed before activate_path() had a chance to run. >> >> Signed-off-by: Hannes Reinecke <hare@xxxxxxx> >> --- >> drivers/md/dm-mpath.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c >> index 0a40fa9..22e7365 100644 >> --- a/drivers/md/dm-mpath.c >> +++ b/drivers/md/dm-mpath.c >> @@ -1196,8 +1196,11 @@ static void activate_path(struct work_struct *work) >> struct pgpath *pgpath = >> container_of(work, struct pgpath, activate_path.work); >> >> - scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev), >> - pg_init_done, pgpath); >> + if (pgpath->is_active) >> + scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev), >> + pg_init_done, pgpath); >> + else >> + pg_init_done(pgpath, SCSI_DH_DEV_OFFLINED); > > Hi Hannes, > > What problem are you going to fix with this? > It is still possible that the path is set to failed just after > "if (pgpath->is_active)" and before "scsi_dh_activate". > activate_path() is run from a workqueue, so there is a race window between the check ->is_active in __pg_init_all_paths() and the time the scsi_dh_activate() is executed. And as activate_path)() is run without any locks we don't have any synchronization with fail_path(). So it's well possible that a path gets failed in between __pg_init_all_paths() and activate_paths(). Granted, we could remove the check for ->is_active in __pg_init_all_paths(), but I wanted to avoid scheduling a workqueue item which is known to be failing. Cheers, Hannes -- Dr. Hannes Reinecke zSeries & Storage hare@xxxxxxx +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- dm-devel mailing list dm-devel@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/dm-devel
