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

[Junichi Nomura]

On 02/04/14 05:28, Mike Snitzer wrote:
> From: Hannes Reinecke <hare at suse.de>
> 
> There is no reason why multipath needs to queue requests internally for
> queue_if_no_path or pg_init; we should rather push them back onto the
> request queue.
> 
> And while we're at it we can simplify the conditional statement in
> map_io() to make it easier to read.
> 
> Since mpath no longer does internal queuing of I/O the table info no
> longer emits the internal queue_size.  Instead it displays 1 if queuing
> is being used or 0 if it is not.
> 
> Signed-off-by: Hannes Reinecke <hare at suse.de>
> Cc: Jun'ichi Nomura <j-nomura at ce.jp.nec.com>
> Signed-off-by: Mike Snitzer <snitzer at redhat.com>

> @@ -391,37 +392,30 @@ static int map_io(struct multipath *m, struct request *clone,
>  
>  	pgpath = m->current_pgpath;
>  
> -	if (was_queued)
> -		m->queue_size--;
> -
> -	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);
> -		m->queue_size++;
> -		if (!m->queue_io)
> -			queue_work(kmultipathd, &m->process_queued_ios);
> -		pgpath = NULL;
> -		r = DM_MAPIO_SUBMITTED;
> -	} else if (pgpath) {
> -		bdev = pgpath->path.dev->bdev;
> -		clone->q = bdev_get_queue(bdev);
> -		clone->rq_disk = bdev->bd_disk;
> -	} else if (__must_push_back(m))
> -		r = DM_MAPIO_REQUEUE;
> -	else
> -		r = -EIO;	/* Failed */
> -
> -	mpio->pgpath = pgpath;
> -	mpio->nr_bytes = nr_bytes;
> -
> -	if (r == DM_MAPIO_REMAPPED && pgpath->pg->ps.type->start_io)
> -		pgpath->pg->ps.type->start_io(&pgpath->pg->ps, &pgpath->path,
> -					      nr_bytes);
> +	if (pgpath) {

> +		if (__pgpath_busy(pgpath))
> +			r = DM_MAPIO_REQUEUE;

Sorry I forgot to comment on this.
The above 2 lines should be removed from this patch.
I suggested to add them because the early version of this patch was
going to remove multipath_busy(). It's no longer true.

Removing them will make this patch easier to understand.

Other than that:

Reviewed-by: Jun'ichi Nomura <j-nomura at ce.jp.nec.com>


> +		else if (pg_ready(m)) {
> +			bdev = pgpath->path.dev->bdev;
> +			clone->q = bdev_get_queue(bdev);
> +			clone->rq_disk = bdev->bd_disk;
> +			mpio->pgpath = pgpath;
> +			mpio->nr_bytes = nr_bytes;
> +			if (pgpath->pg->ps.type->start_io)
> +				pgpath->pg->ps.type->start_io(&pgpath->pg->ps,
> +							      &pgpath->path,
> +							      nr_bytes);
> +		} else {
> +			__pg_init_all_paths(m);
> +			r = DM_MAPIO_REQUEUE;
> +		}
> +	} else {
> +		/* No path */
> +		if (__must_push_back(m))
> +			r = DM_MAPIO_REQUEUE;
> +		else
> +			r = -EIO;	/* Failed */
> +	}
>  
>  	spin_unlock_irqrestore(&m->lock, flags);
>  

-- 
Jun'ichi Nomura, NEC Corporation