Re: [PATCH] dm-mpath: requeue I/O during pg_init

[Hannes Reinecke <hare@suse.de>]

On 11/05/2013 02:02 PM, Alasdair G Kergon wrote:
> On Tue, Oct 01, 2013 at 11:49:56AM +0200, Hannes Reinecke wrote:
>> --- a/drivers/md/dm-mpath.c
>> +++ b/drivers/md/dm-mpath.c
>> @@ -390,7 +390,11 @@ static int map_io(struct multipath *m, struct request *clone,
>>  	if (was_queued)
>>  		m->queue_size--;
>>  
>> -	if ((pgpath && m->queue_io) ||
>> +	if (m->pg_init_required) {
>> +		if (!m->pg_init_in_progress)
>> +			queue_work(kmultipathd, &m->process_queued_ios);
>> +		r = DM_MAPIO_REQUEUE;
>> +	} else if ((pgpath && m->queue_io) ||
>>  	    (!pgpath && m->queue_if_no_path)) {
>>  		/* Queue for the daemon to resubmit */
>>  		list_add_tail(&clone->queuelist, &m->queued_ios);
> 
> This sequence if...else if... is becoming more and more unreadable!
> - Two cases now return REQUEUE; two cases now queue_work().
> 
> If it really can't be simplified, could we at least add some explanatory
> comments inline?
> 
Well, _actually_ this was more an RFC on where would be the point of
having multipath queueing I/Os internally.
This patch remove internal queueing for during pg_init.
I do have a second patch for removing the internal queueing
altogether and drop the entire queue_io stuff.

However, prior to doing so I would like to inquire on the original
design goals. There must have been a reason for implementing the
internal queueing.
If this is just a left-over from the original port to request-based
(for bio-based we _have_ to queue internally as there's no request
queue to be had), fine, we should be removing it.
But the _might_ be some corner cases which require us to do internal
queueing. Junichi should know.

Junichi, could you share some light here?

Cheers,

Hannes
---
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +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)