[dm-devel] [PATCH 8/8] dm-mpath: do not activate failed paths
Junichi Nomura
j-nomura at ce.jp.nec.com
Fri Feb 28 09:37:50 UTC 2014
On 02/28/14 18:32, Hannes Reinecke wrote:
> 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 at suse.de>
>>> ---
>>> 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().
What I pointed is that your patch makes the window very small
but doesn't close it.
If the race is a problem, this patch doesn't fix the problem.
--
Jun'ichi Nomura, NEC Corporation
More information about the dm-devel
mailing list