[dm-devel] [PATCH] dm-mpath: push back requests instead of queueing

[Junichi Nomura]

On 11/12/13 17:49, Hannes Reinecke wrote:
> On 11/12/2013 09:17 AM, Hannes Reinecke wrote:
>> On 11/12/2013 08:48 AM, Junichi Nomura wrote:
>>> On 11/08/13 18:02, Hannes Reinecke wrote:
>>>> @@ -1241,7 +1168,8 @@ static void pg_init_done(void *data, int errors)
>>>>  		m->queue_io = 0;
>>>>  
>>>>  	m->pg_init_delay_retry = delay_retry;
>>>> -	queue_work(kmultipathd, &m->process_queued_ios);
>>>> +	if (!m->queue_io)
>>>> +		dm_table_run_queue(m->ti->table);
>>>>  
>>>>  	/*
>>>>  	 * Wake up any thread waiting to suspend.
>>>
>>> process_queued_ios was responsible for retrying pg_init.
>>> And when retrying, m->queue_io is still 0.
>>> So don't we have to run queue unconditionally here
>>> or call __pg_init_all_paths() directly?

Sorry, I was going to write:
    And when retrying, m->queue_io is still *1*.
    So don't we have to run queue unconditionally here
    or call __pg_init_all_paths() directly?

# Though as far as a request has been requeued, blk_delay_queue
# repeatedly runs it anyway, as the queue isn't stopped..

>> In my rework I've _tried_ to separate both functions from
>> process_queued_ios().
>> But yes, you are right; I haven't considered pg_init_retry.
>> Will be updating the patch.
>>
> Actually, I have. I just had a closer look at the patch,
> and pg_init retry is handled, albeit differently than in
> the original.
> 
> It now works like this:
> 
> ->map_io() is called
>   -> calls '__switch_pg', which sets 'queue_io'
>   -> calls __pg_init_all_paths, which pushes activate_path
>      onto a workqueue
>   -> returns 'MAPIO_REQUEUE'
> 
> -> pg_init_done()
>   -> Checks pg_init_required
>      -> if non-zero some other I/O already
>         kicked off an 'activate_path',
>         so nothing to be done from our side
>      -> if zero we're calling kicking the queue
>         via blk_run_queue
> 
> And blk_run_queue() will be calling into ->request_fn,
> which will pull requests off the queue.
> So on the next request we're calling 'map_io', so the
> entire game starts anew, retrying pg_init.
> 
> The only thing we're not handling properly is the
> 'pg_init_delay_retry', as for that we should've
> started the queue with a certain delay, which
> we currently don't. But that's easily fixable.

-- 
Jun'ichi Nomura, NEC Corporation