[dm-devel] [PATCH 4/7] dm mpath: remove process_queued_ios()
Junichi Nomura
j-nomura at ce.jp.nec.com
Tue Feb 4 09:27:37 UTC 2014
On 02/04/14 18:08, Hannes Reinecke wrote:
> On 02/04/2014 09:55 AM, Junichi Nomura wrote:
>> On 02/04/14 17:18, Hannes Reinecke wrote:
>>>>> @@ -1591,8 +1563,17 @@ static int multipath_ioctl(struct dm_target *ti, unsigned int cmd,
>>>> ...
>>>>> + if (m->current_pg && m->pg_init_required)
>>>>> + __pg_init_all_paths(m, 0);
>>>>> + spin_unlock_irqrestore(&m->lock, flags);
>>>>> + dm_table_run_md_queue_async(m->ti->table);
>>>>> + }
>>>>
>>>> What happens if "!m->current_pg && m->pg_init_required"?
>>>>
>>> >From the current logic it means that no valid pg was found, so
>>> calling pg_init would be pointless.
>>> We're calling __choose_pgpath() before that, so if that returns
>>> with current_pg == NULL all paths are down, and calling
>>> pg_init would be pointless.
>>>
>>> But I think I see to have pg_init_required handling cleared up;
>>> I'll be doing a patch to unset it at the start of __choose_pgpath(),
>>> this we we can be sure that it'll be set correctly.
>>
>> I think it is possible that __choose_pgpath() being called twice
>> before pg_init_required is checked.
>>
>> For example,
>>
>> multipath_ioctl()
>> __choose_pgpath()
>> clear pg_init_required
>> select a new pg
>> __switch_pg()
>> set pg_init_required
>>
>> map_io()
>> __choose_pgpath()
>> clear pg_init_required
>> select the same pg
>> (pg_init_required is not set)
>> ...
>>
> But why should 'map_io' calling __choose_pgpath()?
>
> Either __choose_path() from ioctl was able to set ->current_pg
> (in which case __choose_pgpath() wouldn't be called in map_io)
> or it was not, in which case pg_init_required would need to be reset
> during __choose_pgpath() when called from map_io().
__choose_pgpath() is a function to select "path".
Even if current_pg is set, map_io() may call the function
to select a path. (Please look at the repeat_count check)
So, I suggested the followings:
Call __pg_init_all_paths() regardless of current_pg.
__pg_init_all_paths() returns whether pg_init has started.
If !current_pg, it returns 0.
(https://www.redhat.com/archives/dm-devel/2014-February/msg00013.html)
The semantics is clear:
- if pg_init_required, call __pg_init_all_paths()
- __pg_init_all_paths() might fail to start pg_init (= returns 0).
Then caller should take some action or give up.
--
Jun'ichi Nomura, NEC Corporation
More information about the dm-devel
mailing list